Skip to content

feat(logs,metrics,spans): Add settings object spec for envelope item container#16797

Open
Lms24 wants to merge 2 commits intomasterfrom
lms/feat-develop-container-settings
Open

feat(logs,metrics,spans): Add settings object spec for envelope item container#16797
Lms24 wants to merge 2 commits intomasterfrom
lms/feat-develop-container-settings

Conversation

@Lms24
Copy link
Member

@Lms24 Lms24 commented Mar 5, 2026

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 settings property on the sdk object 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 a boolean instead, 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

@vercel
Copy link

vercel bot commented Mar 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
develop-docs Ready Ready Preview, Comment Mar 5, 2026 2:46pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
sentry-docs Ignored Ignored Preview Mar 5, 2026 2:46pm

Request Review

@Lms24 Lms24 requested review from Dav1dde and cleptric March 5, 2026 14:30
}
```

<SpecSection id="span-envelope-item-settings" status="proposal" since="0.0.0">
Copy link
Member Author

@Lms24 Lms24 Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Lms24 Lms24 marked this pull request as ready for review March 5, 2026 14:34
@Lms24 Lms24 self-assigned this Mar 5, 2026
@linear-code
Copy link

linear-code bot commented Mar 5, 2026

Comment on lines +51 to +52
"infer_ip:": "auto",
"infer_useragent:": "auto"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`.

Copy link
Member

@Dav1dde Dav1dde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment on lines +379 to +380
| `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). |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed with @Dav1dde : We should introduce a top level version field in the item JSON that informs relay what to do with logs:

  • version: 1 is the default. For logs, Relay automatically assumes infer_ip: auto (legacy behavior).
  • New SDK versions set version: 2. Relay assumes infer_ip: never as 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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjbayer to confirm, you'd envision this?

{
  "version": 2,
  "items": [...],
  "settings": {
    "infer_ip:": "auto",
    "infer_useragent:": "auto",
  }
}

Happy to add it!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please!

@Lms24
Copy link
Member Author

Lms24 commented Mar 6, 2026

@Dav1dde

but maybe we wanna bike-shed some more on the namings.

sure, anything that comes to mind? I think ingest_settings was on the table yesterday, which is more explicit and communicates intent more clearly. Happy to change it to that.

Also,

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 a boolean instead, assuming we never need more than two states.

we can "do better" with this second attempt, though tbh this is LOGAF-L for me (bundle size differences are negligible).

@cleptric cleptric self-requested a review March 8, 2026 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants