-
-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add metrics version #1248
base: main
Are you sure you want to change the base?
Add metrics version #1248
Conversation
@@ -11,6 +11,7 @@ import ( | |||
|
|||
var metricsNameRegex = regexp.MustCompile(`\W+`) | |||
var metricsNameFixRegex = regexp.MustCompile(`^_+|_+$`) | |||
var VersionString = "dev" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe could you use https://pkg.go.dev/runtime/debug#ReadBuildInfo instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the version of the FrankenPHP module (and of all other installed modules) should already be exposed by Caddy: https://github.com/caddyserver/caddy/blob/v2.8.4/metrics.go#L15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the string currently output from a production instance:
go_build_info{checksum="",path="github.com/dunglas/frankenphp/caddy",version="(devel)"} 1
It doesn't seem too useful :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum this looks like an issue in my release script 😅 (because of the replace). Isn't the version of lib (not the Caddy module) displayed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean, but that is the only thing in the metrics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we should have the version of github.com/dunglas/frankenphp
too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no dependencies listed, just the top-level.
closes #1225
This adds a new global set by
-X
ld variable,VersionString
which is injected during the Docker build. This can be fetched via thefrankenphp_version
labels, as well as the current php version reported.