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

[bug] Body and the {OriginalFormat} attribute of LogRecord data don't work together when using logging bridge API. #6109

Open
juliuskoval opened this issue Jan 27, 2025 · 4 comments
Labels
bug Something isn't working needs-triage New issues which have not been classified or triaged by a community member pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package

Comments

@juliuskoval
Copy link
Contributor

juliuskoval commented Jan 27, 2025

Package

OpenTelemetry.Exporter.OpenTelemetryProtocol

Package Version

Package Name Version
OpenTelemetry.Exporter.OpenTelemetryProtocal 1.11.0-rc.1

Runtime Version

net9.0

Description

If I populate the Body property of the LogRecordData that I'm trying to emit with the formatted message and create a LogRecordAttributeList with the {OriginalFormat} attribute and pass it to the EmitLog method, the body of the exported log will be the value of {OriginalFormat} and the attribute itself as well as the Body of the LogRecordData won't be there.

Steps to Reproduce

using OpenTelemetry.Logs;

#pragma warning disable OTEL1001
using var provider = Sdk
    .CreateLoggerProviderBuilder()
    .AddOtlpExporter()
    .Build();
    
var logger = provider.GetLogger();

var data = new LogRecordData
{
    Body = "formatted message"
};

var attributes = new LogRecordAttributeList { { "{OriginalFormat}", "original format" } };

logger.EmitLog(data, attributes);

Thread.Sleep(10000);

Expected Result

The body of the exported log will be the Body property of the LogRecordData that I emitted and there will be the {OriginalFormat} attribute.

Actual Result

See description

Additional Context

No response

@juliuskoval juliuskoval added bug Something isn't working needs-triage New issues which have not been classified or triaged by a community member labels Jan 27, 2025
@github-actions github-actions bot added the pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package label Jan 27, 2025
@juliuskoval juliuskoval changed the title [bug] Body and the {OriginalFormat} attribute od LogRecord data don't work together when using logging bridge API. [bug] Body and the {OriginalFormat} attribute of LogRecord data don't work together when using logging bridge API. Jan 27, 2025
@CodeBlanch
Copy link
Member

Question...

var attributes = new LogRecordAttributeList { { "{OriginalFormat}", "original format" } };

Why are you doing this? {OriginalFormat} is an ILogger thing. It has nothing to do with the log bridge API.

Let's say we want to bridge some logging library into OTel. It supports a formatted message and a message template.

This would be more correct usage of the log bridge:

var data = new LogRecordData
{
    Body = "This is a log bridge from some logging library for requestId = 1"
};

var attributes = new LogRecordAttributeList
{ 
   { "dotnet.logging_library_name.template", "This is a log bridge from some logging library for requestId = {RequestId}" }
};

OTLP exporter would send both without issue.

@juliuskoval
Copy link
Contributor Author

I know {OriginalFormat} isn't mentioned in the definition of the log bridge API, but I guess when I was making the NLog library I didn't find a name for log templates in the semantic conventions, so I wanted to mimic the behavior of Microsoft's ILogger to make things consistent between it and NLog. But ultimately I think it's a valid use case regardless of my motivation and the OTLP exporter doesn't allow it.

@CodeBlanch
Copy link
Member

Make your own name 😄 We own the dotnet prefix, really. So dotnet.nlog.template would be fine. Anything with the prefix dotnet.nlog we should be able to drive the semantic conventions.

Strictly speaking, {OriginalFormat} shouldn't be in the OTLP exporter at all. This is a violation of spec! Sort of an evolutionary mistake. dotnet.ilogger.template or something would have been more appropriate and it should be added by the SDK doing the ilogger bridge.

@alanwest What are your thoughts here?

@alanwest
Copy link
Member

But ultimately I think it's a valid use case regardless of my motivation and the OTLP exporter doesn't allow it.

I agree that this is a bug and it should be a valid use case, however it is unlikely something we can resolve in the near term.

I also agree with @CodeBlanch and recommend steering clear of using {OriginalFormat} for your use case. This was an ILogger implementation detail that slipped into the OTLP exporter.

I'm personally not keen on using dotnet. as a prefix. I think something like nlog.template would be more inline with what we have defined for other .NET specific frameworks. For example, there are semantic conventions defined for SignalR which leverages a signalr. prefix for attribute names. That said, since the NLog.Targets.OpenTelemetryProtocol library does not exist within the OpenTelemetry organization, so it's not strictly governed by the semantic conventions, but I would highly recommend using the semantic conventions as a guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-triage New issues which have not been classified or triaged by a community member pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package
Projects
None yet
Development

No branches or pull requests

3 participants