feat(develop-docs): Add timestamp to Metrics API#17791
Draft
philprime wants to merge 1 commit into
Draft
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
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.
DESCRIBE YOUR PR
Problem
The current Metrics API (
Sentry.metrics.count,.gauge,.distribution) always timestamps each metric at the moment the SDK method is called.There is no way for the caller to supply a timestamp.
This is fine when application code emits metrics inline (e.g. incrementing a counter inside a request handler), but it breaks down for an important class of use cases: scraper-style collection, where an external process reads a batch of metrics from a source and then forwards them to Sentry.
Concrete example: Prometheus scraping
A Prometheus-compatible application exposes a
/metricsendpoint that returns all current metric values in a single response:A scraper fetches this snapshot at time T, then iterates through the metrics and calls
Sentry.metrics.gauge(...),Sentry.metrics.count(...), etc. for each one.Without a user-supplied timestamp, each SDK call captures
Date.now()independently.Even on a fast machine, iterating through dozens of metrics introduces sub-millisecond to low-millisecond drift between calls.
This means metrics that were observed at the exact same instant end up with slightly different timestamps.
This change is necessary if we want to extend our sentry-kubernetes integration to support Prometheus-style scraping of metrics in a Kubernetes cluster.
Why this matters: bucket boundaries
Sentry aggregates metrics into time buckets.
When two metrics that logically belong to the same observation are recorded with timestamps close to the bucket boundary, they land in different buckets.
This causes:
Metrics from the same scrape can be close to a bucket boundary, splitting a single observation into two partial data points and causing artificial jitter or gaps on dashboards and correlations.
The probability of hitting a bucket boundary increases with the number of metrics per scrape and the scrape frequency.
Solution
Add an optional
timestampparameter to theoptionsobject of all three metrics methods (count,gauge,distribution).When provided, the SDK uses the caller's timestamp for the metric payload instead of
Date.now().When omitted, behavior is unchanged (current time).
Type
SDKs should accept the language's native date/time type for SDK user ergonomics:
DatedatetimeInstantDateDateTimeInterfaceThe SDK converts to seconds-since-epoch for the wire format, which already has a required
timestampfield.This change should be aligned with the current implementation of the SDKs.
Alternative Approaches
Batching
The wire format already supports batching (the
itemsarray), but the public API is per-metric (count,gauge,distribution).A batch API would be a much larger surface area change than necessary to resolve the issue.
Use
beforeSendMetricto override the timestampThis callback receives the metric object before sending, so it could theoretically mutate the timestamp.
But it runs at flush time (potentially seconds later), not at capture time, and it doesn't have access to the caller's intended timestamp without out-of-band state.
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: