+ opentelemetry: Add metrics support #1289
Open
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.
Adds support for opentelemetry metrics reporting
Main point of discussion from my perspective is whether and how to group by 'component' before sending so that that's accurately captured.. generally I think it's a safe assumption that metrics with the same name will be from the same component, so I'm loath to spend the cycles on verifying that, but OTOH maybe I'm wrong? It could perhaps be a feature toggle down the road if necessary...Removed that bit and just tagging everything with component=kamon-metricsThere's also maybe a little more refactoring that could have been done to share a bit more of the code logic between traces and metrics, but is it really worth it? I reckon we could save about 50 lines or something at most, and it's not like there a whole whack of other telemetry facets out there waiting to be implemented. I think this is ok...
Further Edit: I haven't implemented any sort of automatic adjustment of the offsets for exponential histograms - the value for positive buckets is currently always 1, and negative always -bucketCount. I believe it should be relatively easy to implement automatic shifting of that ( half max scale minus min scale, adjust scales accordingly?) but haven't done so. I also have questions in my mind about the edge case where the original kamon metric bucket 'covers' two or more target buckets. Started to implement some distribution-of-average logic there but have held myself back for now - it should be easy enough to add, though.
There may be some inconstancy in terms of which bucket we allocate frequencies to at boundaries. I'm personally happy however with whatever error term that might introduce, although I admit to not having given it a thorough analysis
I don't know whether the spec would advocate for or against it, but it also seems reasonable to me - whether or not support for balancing base offset is added - to drop any leading 0's in the 'negative' bucket (changing its offset accordingly) and any trailing 0's in the 'positive' bucket.