feat(dev/logs): Add sentry.log.sequence definition to the logs' spec#16492
feat(dev/logs): Add sentry.log.sequence definition to the logs' spec#16492
sentry.log.sequence definition to the logs' spec#16492Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
sentry.log.sequence definition to the logs' specsentry.log.sequence definition to the logs' spec
| The sequence number **MUST NOT**: | ||
| - Be intentionally reset during the process lifetime. |
There was a problem hiding this comment.
You only need it as a discriminator per timestamp, so maybe that's enough as a constraint?
There was a problem hiding this comment.
This might be a rare scenario, but If it does get reset during the lifetime of the SDK process then on environments like Cloudflare it will produce the same timestamps again since the time is frozen throughout their lifetime, so it won't be unique anymore. So, I wanted to close that door.
What do you think?
| "sentry.message.parameter.1": { "value": 85, "type": "integer" }, | ||
| "sentry.message.parameter.2": { "value": 60, "type": "integer" } | ||
| "sentry.message.parameter.2": { "value": 60, "type": "integer" }, | ||
| "sentry.log.sequence": { "value": 0, "type": "integer" } |
There was a problem hiding this comment.
I wonder if this should be top-level, it feels heavily tied to the timestamp and a fundamental attribute of a log.
@k-fish wdyt?
There was a problem hiding this comment.
I namespaced it initially to signal to the user that they should not mess with this (they shouldn't be able anyways).
But yea I agree, it is tied to the logs timestamps heavily and has little to no-use outside of that.
| - Be intentionally reset during the process lifetime. | ||
| - Be used to modify or synthesize sub-millisecond timestamp precision. | ||
|
|
||
| The sequence provides deterministic ordering within a single SDK instance. It does not guarantee ordering across independent processes or workers, which have separate counters. |
There was a problem hiding this comment.
But it does between separate threads?
There was a problem hiding this comment.
Within the same process, yes the counter should be shared across threads. The limitation is only across separate processes or worker isolates where there's no shared memory.
Should I clarify that in the doc?
This pull request updates the telemetry logs specification to introduce deterministic log ordering via a new attribute,
sentry.log.sequence. This change addresses issues in environments where multiple logs can share identical timestamps, ensuring logs can always be ordered correctly.The attribute addresses an issue that came up in Cloudflare’s runtime which freezes all timers and clock functions, causing all timestamp calls to return the same value. This can lead to logs being displayed out of order if they don’t get sent in one request. Instead, they’ll appear in the order of arrival in ingest, which is subject to request latencies and out-of-order request completion.
This is a proposal as we have yet to see what product thinks about this.