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

[Instrumentation.EntityFrameworkCore] Stop emitting db.statement_type attribute #1559

Conversation

Kielek
Copy link
Contributor

@Kielek Kielek commented Feb 1, 2024

Changes

Similar changes to open-telemetry/opentelemetry-dotnet#5301
[Instrumentation.EntityFrameworkCore] Stop emitting db.statement_type attribute

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • [ ] Design discussion issue #
  • [ ] Changes in public API reviewed

@Kielek Kielek added the comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore label Feb 1, 2024
@Kielek Kielek requested a review from a team February 1, 2024 08:43
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 51 lines in your changes are missing coverage. Please review.

Comparison is base (71655ce) 73.91% compared to head (d006a82) 73.40%.
Report is 137 commits behind head on main.

❗ Current head d006a82 differs from pull request most recent head 443cf42. Consider uploading reports for the commit 443cf42 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1559      +/-   ##
==========================================
- Coverage   73.91%   73.40%   -0.52%     
==========================================
  Files         267      256      -11     
  Lines        9615     9453     -162     
==========================================
- Hits         7107     6939     -168     
- Misses       2508     2514       +6     
Flag Coverage Δ
unittests-Exporter.Geneva 58.03% <49.46%> (?)
unittests-Exporter.OneCollector 89.72% <100.00%> (?)
unittests-Extensions 82.75% <100.00%> (?)
unittests-Instrumentation.AspNet 75.96% <ø> (?)
unittests-Instrumentation.EventCounters 75.92% <ø> (?)
unittests-Instrumentation.Owin 85.71% <ø> (?)
unittests-Instrumentation.Process 100.00% <ø> (?)
unittests-Instrumentation.Runtime 100.00% <ø> (?)
unittests-Instrumentation.StackExchangeRedis 71.00% <ø> (?)
unittests-Instrumentation.Wcf 78.47% <ø> (?)
unittests-PersistentStorage 58.80% <ø> (?)
unittests-ResourceDetectors.Azure 80.90% <ø> (?)
unittests-ResourceDetectors.Host 40.00% <ø> (?)
unittests-ResourceDetectors.Process 100.00% <ø> (?)
unittests-ResourceDetectors.ProcessRuntime 76.08% <ø> (?)
unittests-Solution 80.74% <88.57%> (?)

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

Files Coverage Δ
....Exporter.Geneva/GenevaExporterHelperExtensions.cs 100.00% <ø> (+31.81%) ⬆️
...Telemetry.Exporter.Geneva/GenevaExporterOptions.cs 90.00% <ø> (ø)
...OpenTelemetry.Exporter.Geneva/GenevaLogExporter.cs 85.00% <ø> (+7.50%) ⬆️
...lemetry.Exporter.Geneva/GenevaLoggingExtensions.cs 100.00% <ø> (+14.28%) ⬆️
...enTelemetry.Exporter.Geneva/GenevaTraceExporter.cs 82.50% <ø> (+7.50%) ⬆️
...xporter.Geneva/Internal/ConnectionStringBuilder.cs 92.94% <100.00%> (ø)
...ry.Exporter.Geneva/Internal/ExporterEventSource.cs 14.28% <ø> (+9.52%) ⬆️
...eneva/Internal/ReentrantActivityExportProcessor.cs 100.00% <ø> (ø)
...porter.Geneva/Internal/ReentrantExportProcessor.cs 100.00% <ø> (ø)
...ry.Exporter.Geneva/Internal/TableNameSerializer.cs 98.93% <ø> (ø)
... and 91 more

... and 181 files with indirect coverage changes

@Kielek Kielek merged commit cc7eb70 into open-telemetry:main Feb 5, 2024
112 of 113 checks passed
@Kielek Kielek deleted the instrumentation-efcore-drop-db.statement_type branch February 5, 2024 05:45
@julealgon
Copy link

Would be nice if you guys could add a bit more detail on the release notes as to why a change is being made, especially breaking ones.

This is referred in the release notes with just:

Breaking Change: Stop emitting db.statement_type attribute

Even this PR has zero information, and the linked one for the SQL instrumentation is fairly barebones.

@cijothomas
Copy link
Member

Would be nice if you guys could add a bit more detail on the release notes as to why a change is being made, especially breaking ones.

This is referred in the release notes with just:

Breaking Change: Stop emitting db.statement_type attribute

Even this PR has zero information, and the linked one for the SQL instrumentation is fairly barebones.

That is fair feedback.
The reason is - that attribute added at a time when conventions were being written, and the attribute never got part of any conventions. So the change is done to align with the conventions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants