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

Span Name formatting is missing #166

Open
mbedeker opened this issue Dec 12, 2024 · 4 comments
Open

Span Name formatting is missing #166

mbedeker opened this issue Dec 12, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@mbedeker
Copy link

I'm using the Serilog tracing nuget package to add tracing to our framework asp.net mvc application as described here:
serilog-tracing/serilog-tracing#145

Issue is related to this line of code:

Log.Logger.StartActivity(@"{HttpMethod} {RawUrl}", HttpContext.Current.Request.HttpMethod, HttpContext.Current.Request.RawUrl);

Here we start the activity logging the HttpMethod and the RawUrl.

When using this sink to send the traces to an aspire dashboard, the Name property of the span is set to the Message Template, which results in the following on the aspire dashboard.

image

As you can see, the displayed name is not formatted, the property names are still visible. On the left side in the list of spans related to the trace, and also on the right where the properties are displayed.

The properties needed to format the name/message are present with the span.

(the last trace line is generated by a different service, and not with the serilog sink. on that line it is displayed correctly)

I see in the OtlpEventBuilder.cs, that this is done by design?

    public static void ProcessName(Span span, LogEvent logEvent)
    {
        if (!string.IsNullOrWhiteSpace(logEvent.MessageTemplate.Text))
        {
            span.Name = logEvent.MessageTemplate.Text;
        }
    }

Is there a way to configure the sink to also use the formatted message for the spans? I see that there is a feature for formatting the messages of the structured logs, but not the names of the span.

Perhaps a configurable setting to enforce the parsing of the Span name?

@mbedeker mbedeker added the enhancement New feature or request label Dec 12, 2024
@mbedeker
Copy link
Author

I've realized that there is already a way to do this, implemented string interpolation, and it magically worked how it should.

closing the issue now

@nblumhardt
Copy link
Member

Thanks for getting in touch.

String interpolation isn't the intended approach here, since it will result in structured data being lost.

The sink options carry IncludedData flags which control this behavior for log events, see IncludedData.TemplateBody:

https://github.com/serilog/serilog-sinks-opentelemetry/blob/dev/src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/IncludedData.cs#L74

The default for span naming is the opposite, using a template by default, because span names in OTel are expected to be low-cardinality.

I think the best way around this would be to add another IncludedData option, RenderedSpanName or something along those lines. Does that sound like the kind of thing you're in search of?

Thanks!

@mbedeker
Copy link
Author

Hi,

Thanks for the reply, i thought I fixed it the "right" way with string interpolation. but indeed this would in losing the structured data (allthough if i still provide the properties with the StartActivity method, the structured data is still added to the span)

A solution as you suggest would definitely solve any issues we have.

@mbedeker mbedeker reopened this Dec 14, 2024
@nblumhardt
Copy link
Member

Great 👍 thanks for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants