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

[tracing appender] Collect span attributes #1692

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

krfricke
Copy link

@krfricke krfricke commented Apr 30, 2024

Seems related to #1378

Changes

Currently, span attributes are not collected by the OpenTelemetryTracingBridge. E.g. when using

        let my_span =
            tracing::info_span!("my_span", foo = "bar");

        my_span.in_scope(|| {
            tracing::info!(baz = 42, "Log message with metrics");
        });

this will emit the log message with attribute baz = 42, but without foo = bar.

It can still be desirable to emit attribute values, e.g. when not correlating to emitted tracing data.

This PR adds such functionality by adding two flags that control if span attributes are emitted with logs, and if so, if they are flattened or nested.

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

Signed-off-by: Kai Fricke <kai.fricke@openai.com>
Signed-off-by: Kai Fricke <kai.fricke@openai.com>
@krfricke krfricke requested a review from a team as a code owner April 30, 2024 15:17
Copy link

CLA Not Signed

@krfricke
Copy link
Author

I'm reviewing the CLA with my company.

Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

I'd like to hold off from making further changes in this crate until #1689 is resolved to avoid rework.

I do not disagree about this change itself, and we can consider it after the higher order bits are resolved.

@krfricke
Copy link
Author

Sounds good, please feel free to ping me on this PR once that's in place.

@cijothomas
Copy link
Member

Sounds good, please feel free to ping me on this PR once that's in place.

Thanks.
While you wait, we are glad to receive other contributions! Feel free to take a look around and see if you want to help with something.

Since you already have experience with tracing, would be good get your thoughts on #1689 as well!

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.

None yet

2 participants