Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Allow setting TraceContext on opentelemetry::logs::LogRecord #2106

Open
KodrAus opened this issue Sep 11, 2024 · 5 comments · May be fixed by #2129
Open

[Feature]: Allow setting TraceContext on opentelemetry::logs::LogRecord #2106

KodrAus opened this issue Sep 11, 2024 · 5 comments · May be fixed by #2129
Labels
A-log Area: Issues related to logs enhancement New feature or request

Comments

@KodrAus
Copy link
Contributor

KodrAus commented Sep 11, 2024

Related Problems?

No response

Describe the solution you'd like:

The new opentelemetry::logs::LogRecord trait supports a subset of the data on the opentelemetry_sdk::logs::LogRecord struct. One thing it's missing is the ability to set the TraceContext.

This is something I was using to ensure logs emitted via OpenTelemetry would include trace context, even if that context is not managed via OpenTelemetry itself. Since the TraceContext is optional, we should be able to tell when processing the resulting record whether the caller explicitly set a context, and fill it in with OpenTelemetry's current if not.

A method on LogRecord like fn set_trace_context(&mut self, ctxt: TraceContext) would cover my needs.

Considered Alternatives

No response

Additional Context

No response

@KodrAus KodrAus added enhancement New feature or request triage:todo Needs to be traiged. labels Sep 11, 2024
@lalitb
Copy link
Member

lalitb commented Sep 12, 2024

Thanks @KodrAus. this seems valid. do you want to contribute the PR, else I should be able to pick this up sometime next week.

@cijothomas cijothomas added A-log Area: Issues related to logs and removed triage:todo Needs to be traiged. labels Sep 12, 2024
@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 17, 2024

I’m happy to put a PR together for this 👍 I imagine at some point you’ll be locking down those traits so they can be stabilized.

@lalitb
Copy link
Member

lalitb commented Sep 17, 2024

Thanks @KodrAus. Yes, we are targeting RC in Oct, so would have stable API by then,

@KodrAus KodrAus linked a pull request Sep 19, 2024 that will close this issue
4 tasks
@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 19, 2024

I've opened #2129 for this.

As an aside, it was probably discussed somewhere, but I thought the API here was a little surprising. The LogRecord trait appears to mostly be setters for the LogRecord struct so am not sure why it's preferable over using the LogRecord struct itself. It seems like it could be a compatibility hazard going forwards, since you can't introduce new methods without breaking changes, a no-op or panicking default, or forcing additional trait bounds on users of the Logger trait to extend it. None of those options are really appealing in the long term. There could be some bigger plan here I'm missing though since I'm only a casual observer.

@lalitb
Copy link
Member

lalitb commented Sep 19, 2024

The LogRecord trait appears to mostly be setters for the LogRecord struct so am not sure why it's preferable over using the LogRecord struct itself.

The OpenTelemetry API is intentionally minimal, leaving implementation details to the SDK. This gives users the flexibility to bring their own SDKs without being forced into specific design choices. For example, with LogRecord, someone might want to use their own data structure for storing attributes. Or the SDK may want to directly convert the record data in export format without storing it in intermediate ‘LogRecord’ structure. If those details were baked into the API, it would limit their options. By keeping the API simple, we avoid locking users into specific implementations and allow SDKs to handle the details in a way that best suits them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-log Area: Issues related to logs enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants