-
Notifications
You must be signed in to change notification settings - Fork 852
Adding EventName to LogRecord #6306
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #6306 +/- ##
==========================================
+ Coverage 86.74% 86.76% +0.02%
==========================================
Files 258 258
Lines 11879 11881 +2
==========================================
+ Hits 10304 10309 +5
+ Misses 1575 1572 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
69a64d5 to
ff31fb8
Compare
ff31fb8 to
0bca940
Compare
| write position, resulting in gRPC protocol errors. | ||
| ([#6280](https://github.com/open-telemetry/opentelemetry-dotnet/pull/6280)) | ||
|
|
||
| * **Breaking change**: If `EventName` is specified either through `ILogger` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to rephrase this.
- Given this is a stable package, there cannot be breaking changes.
- However, event-name was only exported when enabling the experimental_feature_flag "OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EVENT_LOG_ATTRIBUTES".
The change here is, ILogger's EventName is now exported by default, as LogRecord's EventName. "OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EVENT_LOG_ATTRIBUTES" feature is still required to export EventId.Id as before.
(Not sure of the log-bridge part whether it supported it before or not. If new capability, this is just a feature enhancement, not a breaking change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/OpenTelemetry.Api/CHANGELOG.md
Outdated
|
|
||
| ## Unreleased | ||
|
|
||
| * Added the `EventName` property to `LogRecordData` ([#6306](https://github.com/open-telemetry/opentelemetry-dotnet/pull/6306)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe make it explicit that this feature (entire log bridge) is still under experimental ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
There was a previous conversation on this topic. Please ensure that everything mentioned there is addressed: #4754. |
As far as this goes, I only made it so that |
71ea9a5 to
4267ca3
Compare
| write position, resulting in gRPC protocol errors. | ||
| ([#6280](https://github.com/open-telemetry/opentelemetry-dotnet/pull/6280)) | ||
|
|
||
| * If `EventName` is specified either through `ILogger` or the experimental |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good writeup!
src/OpenTelemetry/Logs/LoggerSdk.cs
Outdated
|
|
||
| logRecord.Data = data; | ||
| logRecord.ILoggerData = default; | ||
| logRecord.ILoggerData.EventId = new EventId(0, data.EventName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Use something to indicate 0 is a conscious default choice.
new EventId(default, data.EventName) does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that makes sense.
cijothomas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. (I recommend to get more eyes before merging, as I am not fully familiar with the experimental bridge side of things.)
src/OpenTelemetry.Api/CHANGELOG.md
Outdated
|
|
||
| ## Unreleased | ||
|
|
||
| * Experimental (only in pre-release versions): Added the `EventName` property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This should be added below the existing entries in the "Unreleased" section
….12.0 to 1.13.1 (#64) Updated [OpenTelemetry.Exporter.OpenTelemetryProtocol](https://github.com/open-telemetry/opentelemetry-dotnet) from 1.12.0 to 1.13.1. <details> <summary>Release notes</summary> _Sourced from [OpenTelemetry.Exporter.OpenTelemetryProtocol's releases](https://github.com/open-telemetry/opentelemetry-dotnet/releases)._ ## 1.13.1 For highlights and announcements pertaining to this release see: [Release Notes > 1.13.1](https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/RELEASENOTES.md#1131). The following changes are from the previous release [1.13.0](https://github.com/open-telemetry/opentelemetry-dotnet/releases/tag/core-1.13.0). * NuGet: [OpenTelemetry v1.13.1](https://www.nuget.org/packages/OpenTelemetry/1.13.1) * Fixed an issue where the Base2 Exponential Bucket Histogram did not reset its scale to 20 after each collection cycle when using delta aggregation temporality. ([#6557](open-telemetry/opentelemetry-dotnet#6557)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.1/src/OpenTelemetry/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Api v1.13.1](https://www.nuget.org/packages/OpenTelemetry.Api/1.13.1) No notable changes. See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.1/src/OpenTelemetry.Api/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Api.ProviderBuilderExtensions v1.13.1](https://www.nuget.org/packages/OpenTelemetry.Api.ProviderBuilderExtensions/1.13.1) No notable changes. See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.1/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Exporter.Console v1.13.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Console/1.13.1) No notable changes. See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.1/src/OpenTelemetry.Exporter.Console/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Exporter.InMemory v1.13.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.InMemory/1.13.1) No notable changes. See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.1/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Exporter.OpenTelemetryProtocol v1.13.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.OpenTelemetryProtocol/1.13.1) No notable changes. See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.1/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Exporter.Zipkin v1.13.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Zipkin/1.13.1) No notable changes. See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.1/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Extensions.Hosting v1.13.1](https://www.nuget.org/packages/OpenTelemetry.Extensions.Hosting/1.13.1) ... (truncated) ## 1.13.1-beta.1 The following changes are from the previous release [1.13.0-beta.1](https://github.com/open-telemetry/opentelemetry-dotnet/releases/tag/coreunstable-1.13.0-beta.1). * NuGet: [OpenTelemetry.Exporter.Prometheus.AspNetCore v1.13.1-beta.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Prometheus.AspNetCore/1.13.1-beta.1) * Updated OpenTelemetry core component version(s) to `1.13.1`. ([#6598](open-telemetry/opentelemetry-dotnet#6598)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.13.1-beta.1/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Exporter.Prometheus.HttpListener v1.13.1-beta.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Prometheus.HttpListener/1.13.1-beta.1) * Updated OpenTelemetry core component version(s) to `1.13.1`. ([#6598](open-telemetry/opentelemetry-dotnet#6598)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.13.1-beta.1/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Shims.OpenTracing v1.13.1-beta.1](https://www.nuget.org/packages/OpenTelemetry.Shims.OpenTracing/1.13.1-beta.1) * Updated OpenTelemetry core component version(s) to `1.13.1`. ([#6598](open-telemetry/opentelemetry-dotnet#6598)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.13.1-beta.1/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md) for details. ## 1.13.0 For highlights and announcements pertaining to this release see: [Release Notes > 1.13.0](https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/RELEASENOTES.md#1130). The following changes are from the previous release [1.12.0](https://github.com/open-telemetry/opentelemetry-dotnet/releases/tag/core-1.12.0). * NuGet: [OpenTelemetry v1.13.0](https://www.nuget.org/packages/OpenTelemetry/1.13.0) * Added a verification to ensure that a `MetricReader` can only be registered to a single `MeterProvider`, as required by the OpenTelemetry specification. ([#6458](open-telemetry/opentelemetry-dotnet#6458)) * Added `FormatMessage` configuration option to self-diagnostics feature. When set to `true` (default is false), log messages will be formatted by replacing placeholders with actual parameter values for improved readability. Example `OTEL_DIAGNOSTICS.json`: ```json { "LogDirectory": ".", "FileSize": 32768, "LogLevel": "Warning", "FormatMessage": true } ``` * Fixed parsing of `OTEL_TRACES_SAMPLER_ARG` decimal values to always use `.` as the delimiter when using the `traceidratio` sampler, preventing locale-specific parsing issues. ([#6444](open-telemetry/opentelemetry-dotnet#6444)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.0/src/OpenTelemetry/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Api v1.13.0](https://www.nuget.org/packages/OpenTelemetry.Api/1.13.0) * Added `AddLink(SpanContext, SpanAttributes?)` to `TelemetrySpan` to support linking spans and associating optional attributes for advanced trace relationships. ([#6305](open-telemetry/opentelemetry-dotnet#6305)) * Experimental (only in pre-release versions): Added the `EventName` property to `LogRecordData` ([#6306](open-telemetry/opentelemetry-dotnet#6306)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.0/src/OpenTelemetry.Api/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Api.ProviderBuilderExtensions v1.13.0](https://www.nuget.org/packages/OpenTelemetry.Api.ProviderBuilderExtensions/1.13.0) No notable changes. See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.13.0/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md) for details. ... (truncated) ## 1.13.0-beta.1 The following changes are from the previous release [1.12.0-beta.1](https://github.com/open-telemetry/opentelemetry-dotnet/releases/tag/coreunstable-1.12.0-beta.1). * NuGet: [OpenTelemetry.Exporter.Prometheus.AspNetCore v1.13.0-beta.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Prometheus.AspNetCore/1.13.0-beta.1) * Updated OpenTelemetry core component version(s) to `1.13.0`. ([#6552](open-telemetry/opentelemetry-dotnet#6552)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.13.0-beta.1/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Exporter.Prometheus.HttpListener v1.13.0-beta.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Prometheus.HttpListener/1.13.0-beta.1) * Updated OpenTelemetry core component version(s) to `1.13.0`. ([#6552](open-telemetry/opentelemetry-dotnet#6552)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.13.0-beta.1/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Shims.OpenTracing v1.13.0-beta.1](https://www.nuget.org/packages/OpenTelemetry.Shims.OpenTracing/1.13.0-beta.1) * Updated OpenTelemetry core component version(s) to `1.13.0`. ([#6552](open-telemetry/opentelemetry-dotnet#6552)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.13.0-beta.1/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md) for details. Commits viewable in [compare view](open-telemetry/opentelemetry-dotnet@core-1.12.0...core-1.13.1). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rene Bentes Pinto <[email protected]>
Fixes #6108
Changes
I added a new property to
LogRecordcalledEventName.Added a field called
EventNameto theLogRecordDatastruct which enables specifyingEventNamewhen using the log bridge API.The OTLP exporter exports
EventNameaccording to the proto definition.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes