-
Notifications
You must be signed in to change notification settings - Fork 780
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
[api-logs] Define OTEL1000 & OTEL1001 diagnostics and decorate APIs #4963
Changes from 11 commits
ab42bd4
87361d6
e5a1401
8ccc64f
843f394
3d470e6
1516d1a
ba9d4ef
2bc3bf2
8b65c4f
b13fac2
d1a3bab
1de3258
ac80204
2ae36f4
5b6281f
7d90823
f76bba1
982bb68
09dec3a
54b5bdc
749ee6e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# OpenTelemetry .NET Diagnostic: OTEL1000 | ||
|
||
## Overview | ||
|
||
This is an experimental feature diagnostic covering the following APIs: | ||
|
||
* `LoggerProviderBuilder` | ||
* `LoggerProvider` | ||
* `IDeferredLoggerProviderBuilder` | ||
|
||
Experimental features may be changed or removed in the future. | ||
|
||
## Details | ||
|
||
The OpenTelemetry Specification defines a `LoggerProvider` as part of its | ||
[API](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/bridge-api.md) | ||
& | ||
[SDK](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md) | ||
components. | ||
|
||
The SDK allows calling `Shutdown` and `ForceFlush` on the `LoggerProvider` and | ||
also allows processors to be added dynamically to a pipeline after its creation. | ||
|
||
Today the OpenTelemetry .NET log pipeline is built on top of the | ||
Microsoft.Extensions.Logging `ILogger` \ `ILoggerProvider` \ `ILoggerFactory` | ||
APIs which do not expose such features. | ||
|
||
We also have an issue with the `ILoggingBuilder.AddOpenTelemetry` API in that it | ||
interacts with the `OpenTelemetryLoggerOptions` class. Options classes are NOT | ||
available until the `IServiceProvider` is available and services can no longer | ||
be registered at that point. This prevents the current logging pipeline from | ||
exposing the same dependency injection surface we have for traces and metrics. | ||
|
||
We are exposing these APIs to solve these issues and gather feedback about their | ||
usefulness. | ||
|
||
## Log Bridge API | ||
|
||
The OpenTelemetry Specification defines a log bridge API which is rooted off of | ||
the `LoggerProvider` (`GetLogger`) and exposes a `Logger` API to submit log | ||
records. See [OTEL1001](.\OTEL1001.md) for details about the log bridge | ||
implementation status. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
# OpenTelemetry .NET Diagnostic: OTEL1001 | ||
|
||
## Overview | ||
|
||
This is an experimental feature diagnostic covering the following APIs: | ||
|
||
* `LoggerProvider.GetLogger` | ||
* `Logger` | ||
* `LogRecordAttributeList` | ||
* `LogRecordData` | ||
* `LogRecordSeverity` | ||
|
||
Experimental features may be changed or removed in the future. | ||
|
||
## Details | ||
|
||
The OpenTelemetry Specification defines a [Logs Bridge | ||
API](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/bridge-api.md). | ||
|
||
The log bridge API is used by library authors to build log appenders which route | ||
messages from different log frameworks into OpenTelemetry. | ||
|
||
Today the OpenTelemetry .NET log pipeline is built on top of the | ||
Microsoft.Extensions.Logging `ILogger` \ `ILoggerProvider` \ `ILoggerFactory` | ||
APIs. | ||
|
||
We are exposing these APIs gather feedback about their usefulness. An | ||
alternative approach may be taken which would be to append into `ILogger` | ||
instead of OpenTelemetry directly. | ||
|
||
## LoggerProvider API | ||
|
||
The OpenTelemetry Specification defines a `LoggerProvider` API. See | ||
[OTEL1000](.\OTEL1001.md) for details about the log implementation status. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,4 +48,3 @@ services: | |
- GF_FEATURE_TOGGLES_ENABLE=traceqlEditor | ||
ports: | ||
- "3000:3000" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unintended? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ya |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// <copyright file="DiagnosticDefinitions.cs" company="OpenTelemetry Authors"> | ||
// Copyright The OpenTelemetry Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
// </copyright> | ||
|
||
#nullable enable | ||
|
||
namespace OpenTelemetry.Internal; | ||
|
||
internal static class DiagnosticDefinitions | ||
{ | ||
public const string UrlFormat = "https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/diagnostics/{0}.md"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe consider an index page? This would allow us to delete the individual md files as experimental features are released or perhaps abandoned. The index page could still contain the master list of features over time and the links to could be adjusted as necessary to a prior tag. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we could ever delete one. Because some shipped code out there will still be pointing to it. All we can do is update say the Should we have individual files or one big one? 🤷 Seems easier to maintain them individually but not sure about that. Looks like StyleCop has 3 levels. An index showing the categories. An index for each category. And then individual files. When I get a warning from StyleCop in VS (just tested CA1311) it links me directly to the individual file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I envisioned there would still be individual files, but they could be deleted and links on the index page could be updated to point to a prior permalinked tag/commit. I too noted that codes for other things (stylecop/xunit/csharp), typically link to a dedicated page. Main reason I was considering a different approach is that experimental features are typically short lived. Though no strong opinion here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK you convinced me this was useful so I implemented it. Take a look at latest and LMK. I also renamed things from "Experimental Feature" to "Experimental API" in the code & docs. I thought that would help differentiate these from the experimental features we typically hide behind envvars. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, looks good! Though bear with me through a little more bikeshedding... I think the name of the folder
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alanwest I pushed some updates. To browse it check out: https://github.com/CodeBlanch/opentelemetry-dotnet/tree/api-log-diagnostics/docs/diagnostics I kept the "diagnostics" folder (didn't rename to "codeanalysis") and I used "README"s instead of "index"s but otherwise I did what you suggested. |
||
|
||
public const string LoggerProviderExperimentalFeature = "OTEL1000"; | ||
public const string LogBridgeApiExperimentalFeature = "OTEL1001"; | ||
} |
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.
We discussed in our meeting a few weeks back anticipating some code taxonomy. A few points of reference for further discussion:
They both take the approach that every 100 or 1000 codes represent a new category of warnings/errors. We could do similar. Another option would be to use a different prefix specifically for feature flags since unlike normal codes, they're ephemeral e.g.,
OTELFF
or something.I have no strong opinions, just sharing additional thoughts.
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 have been thinking about this but so far haven't come up with anything super useful. We could always just say the 1XXX codes are for experimental features and use other codes (eg 2XXX) for some future category?