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

Reintroduce experimental log attributes for 1.7.0-alpha release #4875

Closed
alanwest opened this issue Sep 22, 2023 · 2 comments
Closed

Reintroduce experimental log attributes for 1.7.0-alpha release #4875

alanwest opened this issue Sep 22, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@alanwest
Copy link
Member

As of the the last non-stable release of the OTLP log exporter (1.5.0-rc.1) the following attributes were emitted:

  • dotnet.ilogger.category
  • EventId.Id
  • EventId.Name
  • exception.type
  • exception.message
  • exception.stacktrace

https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.5.0-rc.1/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/LogRecordExtensions.cs#L84-L109

Upon releasing a stable OTLP log exporter (1.6.0) we commented out the above code because the conventions for these attributes remain experimental.

There exist users who want these attributes back.

I propose we reintroduce these attributes in the 1.7.0-alpha release. I recommend we reinstate the code as it was in version 1.5.0-rc.1 and prior gated by an environment variable - e.g. OTEL_DOTNET_EMIT_EXPERIMENTAL_LOG_ATTRIBUTES=true.

While important conversations remain regarding what the attributes should be named, adjusting the name of these attributes is not within the scope of this issue. However, to my knowledge, the most controversial attributes are: dotnet.ilogger.category, EventId.Id, and EventId.Name. I agree these names are unsatisfactory, but due to the fact that they were in existence for quite awhile, I am content reintroducing them as-is for now. If others would be more comfortable leaving these three attributes commented out for now, I also support this stance. It seems possible that the exception.* attributes are desired the most by users at this moment anyways.

We aim to release 1.7.0-alpha next week. If we cannot come to an agreement on this issue then we will move forward with the release without reintroducing these attributes.

@vishweshbankwar please work with your team to come to a consensus on this issue. I am open to other proposals for the 1.7.0-alpha release.

@alanwest alanwest added the enhancement New feature or request label Sep 22, 2023
@cijothomas
Copy link
Member

Given exception is most important, and is purely waiting for OTel semantic convention, could we use a separate opt-in flag for exception ? All others are entirely OTel.NET issues, so I think it makes sense to separate them from semantic convention stability.

OTEL_DOTNET_EMIT_EXCEPTION_LOG_ATTRIBUTES -- for exception attributes.
OTEL_DOTNET_EMIT_EXPERIMENTAL_LOG_ATTRIBUTES -- for everything except exception.

(Also should the env variable indicate this is an OTLPExporter specific one like "OTEL_DOTNET_EXPORTER_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES")

@alanwest
Copy link
Member Author

alanwest commented Sep 22, 2023

OTEL_DOTNET_EMIT_EXCEPTION_LOG_ATTRIBUTES -- for exception attributes.
OTEL_DOTNET_EMIT_EXPERIMENTAL_LOG_ATTRIBUTES -- for everything except exception.

👍 to two feature flags

(Also should the env variable indicate this is an OTLPExporter specific one like "OTEL_DOTNET_EXPORTER_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES")

I've considered this in the past, but I lean towards no. This is because I think there is another important conversation to have: whether these attributes and their names are the responsibility of the OTLP exporter (and any other exporter) to govern how these attributes should be emitted. I generally feel that it is the responsibility of instrumentation to govern attributes emitted not exporters. We've blurred that line for logs, and I think it's still TBD if we can unblur it.

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

3 participants