add strict semantic convention 1.27 support #39
Merged
+253
−55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kpacha
approved these changes
Jan 10, 2025
http/client/transport_metrics.go
Outdated
| metric.WithUnit(v127.HTTPClientRequestDurationUnit), | ||
| metric.WithDescription("Time spent on TLS negotiation and connection"), | ||
| kotelconfig.TimeBucketsOpt) | ||
| } else { |
There was a problem hiding this comment.
avoid the else, just return at the end of the if and let the other branch take control of the rest of the method
http/client/transport_metrics.go
Outdated
| if meter == nil { | ||
| return nil | ||
| } | ||
| type metricFillerFn func(metricsOpts *TransportMetricsOptions, meter metric.Meter, tm *transportMetrics) |
There was a problem hiding this comment.
type definitions don't require named arguments
kpacha
approved these changes
Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables to set
globaloptions at the endpoint level, only for settingstatic_metrics_attributesandstatic_traces_attributes.Since the semantic convention's
http.server.*signals for metrics are reported at the "global" / "service" level , we might want to add per endpoint attributes, that are assigned to those metrics.In order to opt-in for the changes that make those metrics we add a
semantic_conventionparam, that will use the newest names (for now the"1.27"option is the only valid version, a part from""that keeps using the initial set of metrics). The param is only set at the global level, but is taken into account also for each of the backends, to use thehttp.client.*metrics that are in the spec.To decide: in case we "opt-in" for sem conv, what to do with non standarized metrics (the ones that do not have a correlation to standarized names) ? should we not report them anymore, or keep reporting them.
Checked that 1.27 is the same as 1.29 semantic convention for the metrics we are interested in