feat(logs): initial work on develop docs for sdk logging api #12920
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
AbhiPrasad
changed the title
Co-authored-by: Alex Krawiec <[email protected]>
Co-authored-by: Lorenzo Cian <[email protected]>
develop-docs/sdk/telemetry/logs.mdx
Outdated
| { | ||
| "key": "string_item", | ||
| "value": { | ||
| "stringValue": "value" | ||
| } | ||
| }, |
There was a problem hiding this comment.
I find this format quite clunky, is this how OTel does things? If we have some leeway here I think the following would already go a long way towards making this easier to work with:
- removing the extra nesting; i.e., not having
"value"be a dictionary but rather the flat value - instead of having different names for the key depending on the type, moving the type into its own key
So basically the above would become this:
| { | |
| "key": "string_item", | |
| "value": { | |
| "stringValue": "value" | |
| } | |
| }, | |
| { | |
| "key": "string_item", | |
| "value": "value", | |
| "type": "string", | |
| }, |
I'd especially like to do 2. since in Python it otherwise leads to some non-idiomatic code where instead of just accessing the value by key, you have to pop() from the dict instead. It's not a huge deal but I had to work with dicts like this and it felt very clunky.
There was a problem hiding this comment.
Nevermind, this is the format for otel_log so I guess we can't be creative here. :) But maybe we can come up with a nicer format for logs once we get to defining that.
There was a problem hiding this comment.
I was also thinking that the format is a bit verbose. But as this is just the otel logs data model https://opentelemetry.io/docs/specs/otel/logs/data-model/ it is fine.
For the logs type I think we should also be as close to potel as possible. As this is an internal format and the user will probably never see this, it does not matter if it is a bit verbose.
There was a problem hiding this comment.
I think we may be heading towards using AnyValue proto (the value type you're seeing here, from OTel) in our RPCs, in which case I think we may not want to be creative at all for the transport protocol for sdks since AnyValue will be used essentially everywhere in our product from ingest to rpc to frontend.
It may make more sense for the sdks to offer idiomatic apis that convert into our protocol, especially since the idioms differ anyway (eg. stringValue is fine in js since it's usually camelCase).
There was a problem hiding this comment.
We've improved this with logs pageload, see 3ff6e56
| @@ -273,6 +273,7 @@ By default the SDK should set the following attributes: | |||
| 5. `sentry.trace.parent_span_id`: The span id of the span that was active when the log was collected. This should not be set if there was no active span. | |||
| 6. `sentry.sdk.name`: The name of the SDK that sent the log | |||
| 7. `sentry.sdk.version`: The version of the SDK that sent the log | |||
| 8. [BACKEND SDKS ONLY] `server.address`: The address of the server that sent the log. Equivalent to `server_name` we attach to errors and transactions. | |||
There was a problem hiding this comment.
Should this be sentry.server.address?
There was a problem hiding this comment.
We're using the OTEL conventions directly here, so I feel fine with setting it this way. There's nothing specific about this attribute to sentry.
### Summary Now that we've mostly finalized the logs protocol with sdks (see [develop doc for more info](getsentry/sentry-docs#12920)), we want to update Relay to allow the 'log' item type to be sent in this format. SDK's will likely primarily use the `log` ItemType instead of `otel_log` since we don't need to change timestamp conventions, can send a simplified `level` (see `OurLogLevel` added in this PR). #### Schema changes We've deprecated some fields in the protocol that only exist for OTEL: - `severity_number` and `severity_text`, we're coercing these to `level`, but we're keeping the original severity text and number as attributes as OTel allows custom severity text. - `observed_timestamp_nanos` is always set by relay regardless of what is sent because Relay is the 'collector'. We have to leave this as an attribute as well since it's being used by the existing consumer for origin timestamp. - `timestamp_nanos` becomes `timestamp: Timestamp` - `trace_flags`, this is unused, and the consumer doesn't even store it in the table. Will decide what to do with this later. #### Future work - The `ourlog_merge_otel` function can be trimmed down since we won't need to fill in deprecated fields to send the same data to the kafka consumer. - We may need to transform the `OurLog` protocol from json received from sdks to a generic EAP "trace items" kafka message that is essentially a couple fields (eg. traceid) + a KVMap for `attributes`. --------- Co-authored-by: Michi Hoffmann <[email protected]> Co-authored-by: David Herberth <[email protected]>
| *None* | ||
| `item_count` | ||
|
|
||
| : **integer, required.** The number of log items in the envelope. |
There was a problem hiding this comment.
| : **integer, required.** The number of log items in the envelope. | |
| : **integer, required.** The number of log entries in the item. |
It's just scoped to the item, unfortunately naming clashes here "log item" and "envelope item"
There was a problem hiding this comment.
Naming is hard - good catch, will fix.
|
This is ready to merge, will do so as soon as CI passes. The only thing major TODO to iterate on this the default attributes section. Because this still has some open Q, I'm going to tackle this in a follow up. |
Bundle ReportChanges will increase total bundle size by 135 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
resolves #12870
Sentry is exploring an application logging product! More details here in our internal notion: https://www.notion.so/sentry/PRFAQ-Application-Logging-1258b10e4b5d80ea8e98e8ec5505542c.
To get the ball rolling, I'm helping push forward the spec in the SDK. This was initially in Notion (https://www.notion.so/sentry/Logs-in-the-SDK-1a48b10e4b5d80b1827fe14d9fd0236b#1a58b10e4b5d80019d2eef6d84213d4a), but now we're moving it to develop so that we can work more in the open.
This PR documents:
You'll notice only theNo longer appliesotel_logenvelope item is documented. This is what we used for the first alpha release of the SDK, but we'll be moving the SDKs to use thelogenvelope item instead. Until thelogenvelope item protocol is finalized we are avoiding documenting in develop, but we marked it as TODO in the necessary sections.logenvelope item has been documented.Rendered Preview: