feat(logs,metrics,spans): Add settings object spec for envelope item container#16797
feat(logs,metrics,spans): Add settings object spec for envelope item container#16797
settings object spec for envelope item container#16797Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
| } | ||
| ``` | ||
|
|
||
| <SpecSection id="span-envelope-item-settings" status="proposal" since="0.0.0"> |
There was a problem hiding this comment.
spans docs are not yet converted to the changelog-based SpecSection format. Hence for now I added the component but set version 0.0.0.
| "infer_ip:": "auto", | ||
| "infer_useragent:": "auto" |
There was a problem hiding this comment.
Bug: The JSON examples in the documentation for infer_ip and infer_useragent incorrectly include a trailing colon in the field names, making them invalid.
Severity: CRITICAL
Suggested Fix
Remove the trailing colons from the infer_ip: and infer_useragent: field names in the JSON examples within logs.mdx, metrics.mdx, and span-protocol.mdx. The correct format should be "infer_ip": "auto" and "infer_useragent": "auto".
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: develop-docs/sdk/telemetry/spans/span-protocol.mdx#L51-L52
Potential issue: The documentation introduces JSON examples where the field names
`infer_ip` and `infer_useragent` are written with a trailing colon (e.g., `"infer_ip:":
"auto"`). This contradicts the specification tables in the same documents and existing
patterns. If an SDK developer implements this literally, the ingestion service will
silently ignore these invalid fields. This would prevent IP address and user agent
inference from working as intended for SDKs that rely on this documentation. The issue
is present in `logs.mdx`, `metrics.mdx`, and `span-protocol.mdx`.
There was a problem hiding this comment.
Looks good to me, but maybe we wanna bike-shed some more on the namings.
We also need to figure out how this works for logs and the existing extractions, I want to make sure we don't build more special cases.
It also has potential billing impact for logs, as these are billed by size (before extraction).
| | `infer_ip` | String | **OPTIONAL** | The setting to infer the IP address from the incoming request. One of `auto` or `never` (default). | | ||
| | `infer_useragent` | String | **OPTIONAL** | The setting to infer the user agent from the incoming request. One of `auto` or `never` (default). | |
There was a problem hiding this comment.
Discussed with @Dav1dde : We should introduce a top level version field in the item JSON that informs relay what to do with logs:
version: 1is the default. For logs, Relay automatically assumesinfer_ip: auto(legacy behavior).- New SDK versions set
version: 2. Relay assumesinfer_ip: neveras the default.
Without the version field, we'd have to make infer_ip: auto the default, which would force new SDKs to explicitly send infer_ip: never to disable it.
There was a problem hiding this comment.
@jjbayer to confirm, you'd envision this?
{
"version": 2,
"items": [...],
"settings": {
"infer_ip:": "auto",
"infer_useragent:": "auto",
}
}Happy to add it!
sure, anything that comes to mind? I think Also,
we can "do better" with this second attempt, though tbh this is LOGAF-L for me (bundle size differences are negligible). |
This PR adds a specification for ingestion settings of what data Relay should infer from incoming requests. This object shall be the same across our new telemetry items (logs, metrics and spans (v2)) and it lives on the container object:
{ "items": [...], "settings": { "infer_ip:": "auto", "infer_useragent:": "auto", } }This shall serve as a replacement for the
settingsproperty on thesdkobject on events as defined here.I went with the same
'auto'and'never'values but omitted'legacy'since it doesn't seem like we need it. If reviewers prefer, we can also use abooleaninstead, assuming we never need more than two states.Prior to this PR, @cleptric @Dav1dde and I discussed this format. Happy to adjust as reviewers see fit!
ref https://linear.app/getsentry/issue/FE-714/how-to-send-sdksettings-infer-ip