1.0.0 Release

[nblumhardt]

We're aiming to merge this on or around 5th June 2023, once we've gathered some feedback from a month-long release candidate period.

What's in v1.0.0?

• Full support for the OTLP Logs protocol with gRPC and HTTP/protobuf transport flavors
• Full support for structured log events including complex property types
• Follows standard semantic conventions for service naming, exception information, etc.
• Configuration of additional resource attributes
• Configuration of outbound HTTP request handling and attached headers
• Batched/asynchronous WriteTo support
• Synchronous/hard-failing AuditTo support
• Trace and span correlation through System.Diagnostics.Activity

Credits

This release is made possible by:

• @loomis - initial version, build-out, and stabilization
• @nblumhardt - build-out and stabilization
• @Shatl - Reduce memory allocations in HttpExporter #21 - reduce memory allocations
• @maciejpiwowarczyk-relativity - Ability to disable batching #40 - initial batching configuration that led to full AuditTo support
• @SimonCropp - Cleanup #39 - project setup improvements and code style updates

[nblumhardt]

Just thinking some more about stabilizing - perhaps from here, now, we slow down a bit and aim to get at least one maintainer review on every PR? To avoid blocking up the works we could also set a 7-day maximum wait time, so if there's no review within 7 days it's as good as an "Approve".

What do you think @loomis @SimonCropp ?

[SimonCropp]

@nblumhardt i am ok with that.

[loomis]

@nblumhardt Good for me too.

[Kielek]

Hi,

at the beginning I would like to thank for bringing Serilog to OpenTelemetry. Sorry for late feedback, I was not aware about this package earlier.

Based on what I see you have decided to fully implement this feature without relaying on available OpenTelemetry SDK/API.

It will probably leads to some incompatibilities/duplicities of work between this project and the OpenTelemetry SDK/API.

Some examples:

• Implementing OpenTelemetry Protocol Exporter here and in https://github.com/open-telemetry/opentelemetry-dotnet/tree/d5129b215d9d5869ca99046bec3da874b569351a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol.Logs
• Discussions about required resources: Set default service.name and telemetry.sdk.* group according to spec #77 - it could be handled by the OTel SDK/API and https://github.com/open-telemetry/opentelemetry-dotnet/blob/d5129b215d9d5869ca99046bec3da874b569351a/src/OpenTelemetry/Resources/ResourceBuilder.cs#L72-L76
• Lack of support for already implemented Resource Detectors like https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/da2fe978c3cf9c8645d6e1b163e6f0763127df83/src/OpenTelemetry.ResourceDetectors.Container
• Lack of support for non OTLP protocol exporters. If Serilog user would like to use it, it has to be implemented also here.
• Correlation between activities and logs.

To avoid this, the implementation could relay on some bridge API https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/bridge-api.md to keep it in sync.

It is probably to late to introduce changes into 1.0 release, but it is worth to consider it for further usage.

BTW package name you have chosen is a bit misleading and can be probably easily changed.
It is not the sink to the OpenTelemetry, it is rather OpenTelemetryProtocol sink.

Consider to change name from Serilog.Sinks.OpenTelemetry to Serilog.Sinks.OpenTelemetryProtocol.

At the end. it is worth to mention initiative started some time ago by @CodeBlanch. https://github.com/open-telemetry/opentelemetry-dotnet/blob/main-logs/src/OpenTelemetry.Extensions.Serilog

I am not sure what are the plans for this, but it is worth to sync before the release.

[nblumhardt]

Thanks for sharing your thoughts, @Kielek.

at the beginning I would like to thank for bringing Serilog to OpenTelemetry.

Thanks again! To be fair, this is probably the other way around, we're bringing OpenTelemetry to Serilog :-)

Based on what I see you have decided to fully implement this feature without relaying on available OpenTelemetry SDK/API.

Yes, that's correct. Doing this keeps options open for TFM reach, substantially cuts the dependency/deployment footprint of the sink, keeps the functioning of the sink consistent with others in the Serilog ecosystem, and helps us to iterate faster than we otherwise could.

It will probably leads to some incompatibilities/duplicities of work between this project and the OpenTelemetry SDK/API.

Some examples:

Implementing OpenTelemetry Protocol Exporter here and in [...]

So far we've only needed a handful of lines of code for OTLP support; gRPC/protobuf really does the heavy lifting for us here. It was a great choice for the protocol, I think 👍

Discussions about required resources: [...] - it could be handled by the OTel SDK/API and [...]

This seems like the role of the spec, or will be in most cases.

Lack of support for already implemented Resource Detectors like [...]

We're not aiming for feature parity with the OpenTelemetry SDK. It might help to view this sink in the context of Serilog itself, which then might be more comparable to the SDK.

In the case you're describing, Serilog already has an enrichment system that covers the same kinds of scenarios. The equivalent of a resource detector would be a Serilog enricher; as an example, our path to this kind of functionality will more likely integrate with the existing Serilog features (making the enriched log data available through other sinks, and not just this one).

Lack of support for non OTLP protocol exporters. If Serilog user would like to use it, it has to be implemented also here.

That's incorrect. Serilog has a large, stable sink ecosystem that covers this well. If a Serilog user wants to use a different log target, they plug the appropriate sink into Serilog, and don't need any dependency on this sink.

Again it probably helps to view this in the context of Serilog itself. This sink is an exporter, in its role as part of Serilog.

Correlation between activities and logs.

The sink collects trace and span ids from the current System.Diagnostics.Activity when available, and includes them in log records using OTLP's trace and span id fields, which achieves this as far as I'm aware.

To avoid this, the implementation could relay on some bridge API [...] to keep it in sync.

It is probably to late to introduce changes into 1.0 release, but it is worth to consider it for further usage.

We're not eager to take a dependency on the SDK for reasons mentioned earlier.

BTW package name you have chosen is a bit misleading and can be probably easily changed.

It is not the sink to the OpenTelemetry, it is rather OpenTelemetryProtocol sink.

Consider to change name from Serilog.Sinks.OpenTelemetry to Serilog.Sinks.OpenTelemetryProtocol.

I think the name we have is the one users will look for; the protocol/SDK/bridge API distinction is pretty subtle. Thanks for your feedback, though.

At the end. it is worth to mention initiative started some time ago by @CodeBlanch. [...]

I am not sure what are the plans for this, but it is worth to sync before the release.

I'd love to run through OpenTelemetry.Extensions.Serilog with @CodeBlanch and help spot any missed opportunities there. I did check in on this early on, but it seemed more like a proof-of-concept at that point, rather than something intended to be stabilized (apologies if I got this wrong). I've worked on a stack of "Serilog-to-some-other-model" kinds of scenarios, so definitely sign me up if I can help, there.

[pellared]

Feedback after ~30min review:

The approach proposed is NOT compliant with OpenTelemetry specification.

The OpenTelemtry SDK should accept a logger provider via Bridge API which is not done yet (see: open-telemetry/opentelemetry-dotnet#4433)

As far as I understand correctly, the Serilog Sink ("log appender" in OpenTelemetry specification) would only have to depend on OpenTelemetry.Api NuGet package.

P.S. @CodeBlanch would probably be the best person to cooperate with 😉

[nblumhardt]

The approach proposed is NOT compliant with [...].

@pellared I'm sorry, did one of us come to your issue tracker and start shouting unsolicited advice? Please keep the discourse constructive.

Your post only repeats what has already been discussed earlier in the thread.

P.S. @CodeBlanch would probably be the best person to cooperate with 😉

You'll notice that Mikel is already looped in on this thread above.

[pellared]

Please keep the discourse constructive. Your post only repeats what has already been discussed earlier in the thread.

@nblumhardt Sorry, I did my best to do it. I haven't found any explicit mention that currently implemented approach is not specification compliant which is my biggest concern. Maybe it could be at least mentioned in the docs?

[nblumhardt]

@pellared that's fine - thanks 🙂

We try to spell out what the sink does in the first line of the README:

This Serilog sink will transform Serilog events into OpenTelemetry LogRecords and send them as a protobuf payload to an OpenTelemetry OTLP (gRPC or HTTP) endpoint.

I'll make some updates to improve the wording, there. Don't get me wrong, we're 100% aiming for protocol spec compliance - I think where things stand right now we might even be one of the most compliant implementations of the logs protocol that's available on NuGet! 😎

The 1.0.0 release PR might not be the right place to kick off further collaboration, it's already getting rather lengthy 😅

[CodeBlanch]

Just for anyone curious...

Over in https://github.com/open-telemetry/opentelemetry-dotnet I'm working on implementing the OpenTelemetry Specification Bridge API/SDK: open-telemetry/opentelemetry-dotnet#4433

Once that is done and made public I'll update and make available OpenTelemetry.Extensions.Serilog (and OpenTelemetry.Extensions.EventSource). Those links are to a branch I did last year before the spec was stable as part of the effort to validate the design.

Hoping to ship previews in the next few months and then stable by the end of the year.

The one challenge with OTel .NET & Serilog is the SDK doesn't really have a good way at the moment to represent deep object graphs. Serilog has the destructuring thing. My plan is the initial release will just support top-level fields and then I'm hoping to solve this gap in the SDK (somehow) and then update the bridge.

@nblumhardt I have no issue with this sink existing. It will probably provide better support of the full breadth of what Serilog can do (initially and maybe forever 😄)! One suggestion though: Consider naming the package Serilog.Sinks.OpenTelemetryProtocol. That is probably a little more representative of what it is doing.

[nblumhardt]

Hi @CodeBlanch, thanks for dropping by!

It's great to hear that OpenTelemetry.Extensions.Serilog is going ahead; is there any chance the project might be made a bit easier to find and follow in the near future? I'm sure that many of us here (me included) would enjoy contributing to that effort as well.

Also, is there a roadmap or TODO list somewhere we can check out? I.e. are things like full structured data support on the cards? And we should definitely chat about how attributes that are important to Serilog (particularly message templates) can be handled in the resulting log records.

It'd be awesome to see OpenTelemetry.Extensions.Serilog grow and stabilize; if/when you're keen for more eyes on it, please let me/us know where the best place to get involved is. All the best with it!

[nblumhardt]

Time to run the build and shake out any last publishing problems. Thanks again to everyone who contributed and provided feedback!