[api-logs] Define OTEL1000 & OTEL1001 diagnostics and decorate APIs #4963

CodeBlanch · 2023-10-18T21:35:03Z

Relates to #4433

Changes

Added diagnostic definitions for OTEL1000 & OTEL1001.
Decorated experimental log APIs in OpenTelemetry.Api using the new .NET 8 ExperimentalAttribute.

Details

I decided to split things into two categories:

OTEL1000: LoggerProvider & LoggerProviderBuilder
OTEL1001: Log Bridge API

I did that because I think we may want to release the first category stable before the other. More details can be found in the md files I added.

….Api using .NET8 ExperimentalAttribute.

docs/diagnostics/OT1000.md

codecov · 2023-10-19T23:19:29Z

Codecov Report

Merging #4963 (749ee6e) into main (93fd7d3) will decrease coverage by 0.09%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4963      +/-   ##
==========================================
- Coverage   83.42%   83.33%   -0.09%     
==========================================
  Files         297      297              
  Lines       12380    12380              
==========================================
- Hits        10328    10317      -11     
- Misses       2052     2063      +11

Flag	Coverage Δ
unittests	`83.33% <ø> (-0.09%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...c/OpenTelemetry.Api/Logs/LogRecordAttributeList.cs	`68.14% <ø> (ø)`
src/OpenTelemetry.Api/Logs/LogRecordData.cs	`100.00% <ø> (ø)`
...nTelemetry.Api/Logs/LogRecordSeverityExtensions.cs	`100.00% <ø> (ø)`
src/OpenTelemetry.Api/Logs/Logger.cs	`100.00% <ø> (ø)`
src/OpenTelemetry.Api/Logs/LoggerProvider.cs	`100.00% <ø> (ø)`
...rc/OpenTelemetry.Api/Logs/LoggerProviderBuilder.cs	`100.00% <ø> (ø)`

... and 7 files with indirect coverage changes

github-actions · 2023-10-28T03:16:31Z

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

…pentelemetry-dotnet into api-log-diagnostics

martinjt · 2023-11-09T22:21:38Z

docs/diagnostics/OTEL1000.md

+We are exposing these APIs to solve these issues and gather feedback about their
+usefulness.
+
+## Log Bride API


I'm fairly confident you mean "bridge" not "bride"

Thanks @martinjt fixed 👰‍♀️ -> 🌉

I'm fairly confident you mean "bridge" not "bride"

Way to ruin a perfectly serviceable proposal to the logging API 😏

alanwest · 2023-11-10T22:26:43Z

src/Shared/DiagnosticDefinitions.cs

+
+internal static class DiagnosticDefinitions
+{
+    public const string UrlFormat = "https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/diagnostics/{0}.md";


Maybe consider an index page?

https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/experimentat-features/README.md#{0}

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.

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 OTEL1000.md file to note at the top (something like): "This was released as stable in vX.Y.Z and is no longer marked experimental going forward." Or perhaps: "This feature was completely removed in vX.Y.Z and is no longer available going forward."

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.

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.

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.

Cool, looks good! Though bear with me through a little more bikeshedding...

I think the name of the folder diagnostics may get confused with other things. I suggest an organization like:

docs/ └── codeanalysis/ ├── experimental-apis/ │ ├── index.md │ ├── OTEL1000.md │ └── OTEL1001.md └── some-future-category/ └── OTEL2000.md

@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.

alanwest · 2023-11-16T21:50:52Z

build/Common.props

@@ -9,6 +9,8 @@
    <GenerateDocumentationFile>true</GenerateDocumentationFile>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
+    <!-- Suppress warnings for repo code using experimental features -->
+    <NoWarn>$(NoWarn);OTEL1000;OTEL1001</NoWarn>


We discussed in our meeting a few weeks back anticipating some code taxonomy. A few points of reference for further discussion:

StyleCop

xUnit

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.

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?

cijothomas · 2023-11-17T02:56:50Z

docs/metrics/exemplars/docker-compose.yaml

@@ -48,4 +48,3 @@ services:
      - GF_FEATURE_TOGGLES_ENABLE=traceqlEditor
    ports:
      - "3000:3000"
-


unintended?

Ya ☹️ I had this file modified locally for a demo I was doing and it got included. I can't seem to get the diff to eliminate it completely.

…pentelemetry-dotnet into api-log-diagnostics

docs/diagnostics/EXPERIMENTAL_APIS.md

github-actions · 2023-11-26T03:16:42Z

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

github-actions · 2023-12-07T03:16:50Z

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

CodeBlanch added 2 commits October 18, 2023 13:37

Define OT1000 & OT1001 diagnostics and decorate APIs in OpenTelemetry…

ab42bd4

….Api using .NET8 ExperimentalAttribute.

Revert accidental changes.

87361d6

CodeBlanch requested a review from a team October 18, 2023 21:35

reyang reviewed Oct 18, 2023

View reviewed changes

docs/diagnostics/OT1000.md Outdated Show resolved Hide resolved

Merge branch 'main' into api-log-diagnostics

e5a1401

github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Oct 28, 2023

CodeBlanch added 4 commits November 1, 2023 15:42

Code review.

8ccc64f

Merge from main.

843f394

Merge branch 'api-log-diagnostics' of https://github.com/CodeBlanch/o…

3d470e6

…pentelemetry-dotnet into api-log-diagnostics

Warning cleanup.

1516d1a

CodeBlanch changed the title ~~[api-logs] Define OT1000 & OT1001 diagnostics and decorate APIs~~ [api-logs] Define OTEL1000 & OTEL1001 diagnostics and decorate APIs Nov 1, 2023

github-actions bot removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Nov 2, 2023

martinjt reviewed Nov 9, 2023

View reviewed changes

CodeBlanch added 3 commits November 13, 2023 09:55

Merge branch 'main' into api-log-diagnostics

ba9d4ef

Typo fix.

2bc3bf2

Merge branch 'main' into api-log-diagnostics

8b65c4f

alanwest reviewed Nov 16, 2023

View reviewed changes

utpilla approved these changes Nov 16, 2023

View reviewed changes

Merge branch 'main' into api-log-diagnostics

b13fac2

cijothomas reviewed Nov 17, 2023

View reviewed changes

CodeBlanch added 6 commits November 16, 2023 20:57

Code review.

d1a3bab

Code review.

1de3258

Tweaks.

ac80204

Merge branch 'api-log-diagnostics' of https://github.com/CodeBlanch/o…

2ae36f4

…pentelemetry-dotnet into api-log-diagnostics

Rename landing doc to be plural.

5b6281f

Lint.

7d90823

CodeBlanch commented Nov 18, 2023

View reviewed changes

docs/diagnostics/EXPERIMENTAL_APIS.md Outdated Show resolved Hide resolved

Update docs/diagnostics/EXPERIMENTAL_APIS.md

f76bba1

github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Nov 26, 2023

alanwest approved these changes Nov 29, 2023

View reviewed changes

github-actions bot removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Nov 30, 2023

github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Dec 7, 2023

CodeBlanch added 3 commits December 7, 2023 10:27

Merge remote-tracking branch 'upstream/main' into api-log-diagnostics

982bb68

Copyright header updates.

09dec3a

Code review.

54b5bdc

github-actions bot removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Dec 8, 2023

Merge branch 'main' into api-log-diagnostics

749ee6e

CodeBlanch merged commit f5c1160 into open-telemetry:main Dec 8, 2023
78 checks passed

CodeBlanch deleted the api-log-diagnostics branch December 8, 2023 17:31

CodeBlanch mentioned this pull request Dec 8, 2023

[api-extensions-logs] ExperimentalAttribute decoration for experimental APIs #5148

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[api-logs] Define OTEL1000 & OTEL1001 diagnostics and decorate APIs #4963

[api-logs] Define OTEL1000 & OTEL1001 diagnostics and decorate APIs #4963

CodeBlanch commented Oct 18, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading

github-actions bot commented Oct 28, 2023

martinjt Nov 9, 2023

CodeBlanch Nov 13, 2023

terrajobst Nov 16, 2023

alanwest Nov 10, 2023

CodeBlanch Nov 16, 2023

alanwest Nov 16, 2023

CodeBlanch Nov 17, 2023

alanwest Nov 18, 2023

CodeBlanch Dec 7, 2023

alanwest Nov 16, 2023

CodeBlanch Nov 16, 2023

cijothomas Nov 17, 2023

CodeBlanch Nov 17, 2023

github-actions bot commented Nov 26, 2023

github-actions bot commented Dec 7, 2023

[api-logs] Define OTEL1000 & OTEL1001 diagnostics and decorate APIs #4963

[api-logs] Define OTEL1000 & OTEL1001 diagnostics and decorate APIs #4963

Conversation

CodeBlanch commented Oct 18, 2023 • edited Loading

Changes

Details

codecov bot commented Oct 19, 2023 • edited Loading

Codecov Report

github-actions bot commented Oct 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 26, 2023

github-actions bot commented Dec 7, 2023

CodeBlanch commented Oct 18, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading