10000 [sdk-metrics] Support .NET 9 Synchronous Gauge by rajkumar-rangaraj · Pull Request #5867 · open-telemetry/opentelemetry-dotnet · GitHub
[go: up one dir, main page]

Skip to content

Conversation

rajkumar-rangaraj
Copy link
Contributor
@rajkumar-rangaraj rajkumar-rangaraj commented Sep 27, 2024

Fixes #4805
Design discussion issue #

Changes

Please provide a brief description of the changes here.

Updated the Metric class to add support for synchronous gauge.

Follow up: Update document - #5868

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@github-actions github-actions bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Sep 27, 2024
Copy link
codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.28%. Comparing base (6250307) to head (8539901).
Report is 333 commits behind head on main.

Files with missing lines Patch % Lines
src/OpenTelemetry/Metrics/Metric.cs 80.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5867      +/-   ##
==========================================
+ Coverage   83.38%   86.28%   +2.89%     
==========================================
  Files         297      257      -40     
  Lines       12531    11214    -1317     
==========================================
- Hits        10449     9676     -773     
+ Misses       2082     1538     -544     
Flag Coverage Δ
unittests ?
unittests-Project-Experimental 86.26% <81.81%> (?)
unittests-Project-Stable 85.95% <81.81%> (?)

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

Files with missing lines Coverage Δ
src/OpenTelemetry/Metrics/Reader/MetricReader.cs 87.82% <100.00%> (ø)
src/OpenTelemetry/Metrics/Metric.cs 96.89% <80.00%> (+0.37%) ⬆️

... and 231 files with indirect coverage changes

@rajkumar-rangaraj rajkumar-rangaraj marked this pull request as ready for review September 27, 2024 04:25
@rajkumar-rangaraj rajkumar-rangaraj requested a review from a team as a code owner September 27, 2024 04:25
Copy link
Member
@reyang reyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, would you also update the docs? For example

| [Gauge](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#gauge) (experimental) | N/A |

Copy link
Member
@Kielek Kielek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Do you know if it should trigger any changes on instrumentation (packages) side?
Contrib repository/natively instrumented packages?

Copy link
Member
@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good.
Requesting changes to ensure the test covers a corner case with Gauge and Temporality, see https://github.com/open-telemetry/opentelemetry-dotnet/pull/5867/files#r1778773598

@github-actions github-actions bot added the documentation Documentation related label Sep 27, 2024
@rajkumar-rangaraj
Copy link
Contributor Author

Q: Do you know if it should trigger any changes on instrumentation (packages) side? Contrib repository/natively instrumented packages?

Not yet, need to investigate it.

@CodeBlanch CodeBlanch changed the title [sdk-metrics] Add support .NET 9 Synchronous Gauge [sdk-metrics] Support .NET 9 Synchronous Gauge Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation related pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support .NET 9 Synchronous Gauge
5 participants
0