fix: enforce single provider init telemetry sample - #537
Open
nicklasl wants to merge 1 commit into
Open
Conversation
This file contains hidden or 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
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.
Context
#485 introduced
provider_init_ratetelemetry for the local providers. The provider wrapper is the sole owner of this metric: one provider instance should emit exactly one sample, withcount = 1, on its first successful telemetry write.The initial implementation appended that sample to the repeated protobuf field. That happened to work when the resolver returned an empty
provider_init_rate, but it encoded the wrong merge behavior: any unexpected or stale samples from a lower-level resolver would be retained, producing multiple provider-init samples in one request. The JS test also codified that behavior by expecting the existing sample to survive.Counts greater than one should only appear after the telemetry backend aggregates samples from multiple provider initializations. A provider should never send a single wire sample with an accumulated count, nor combine its sample with samples supplied by a lower layer.
Change
Make the provider layer replace
provider_init_ratewith a singleton sample instead of appending to it in the JS, Java, Go, Python, and Rust local providers:Other telemetry metadata in the request, including resolver version and SDK information, is still preserved or populated as before. The existing once-only and retry behavior is unchanged; this only makes ownership of the field explicit when the first sample is attached.
Each provider now has regression coverage that starts with an existing provider-init sample and verifies that it is replaced by exactly one provider-owned sample.
Validation
ProviderTelemetryResolverTestand Spotify Java formatter