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

[repo] Fix the solution build in CI to fail if errors are thrown #1487

Merged
merged 9 commits into from
Dec 13, 2023

Conversation

CodeBlanch
Copy link
Member

nt

Copy link

codecov bot commented Dec 11, 2023

Codecov Report

Merging #1487 (7a81f31) into main (71655ce) will decrease coverage by 0.63%.
Report is 95 commits behind head on main.
The diff coverage is 54.44%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1487      +/-   ##
==========================================
- Coverage   73.91%   73.29%   -0.63%     
==========================================
  Files         267      253      -14     
  Lines        9615     9388     -227     
==========================================
- Hits         7107     6881     -226     
+ Misses       2508     2507       -1     
Flag Coverage Δ
unittests-Exporter.Geneva 57.92% <32.14%> (?)
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.ProcessRuntime 76.08% <ø> (?)
unittests-Solution 80.45% <90.62%> (?)

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 103 more

... and 166 files with indirect coverage changes

@CodeBlanch CodeBlanch marked this pull request as ready for review December 12, 2023 22:35
@CodeBlanch CodeBlanch requested a review from a team December 12, 2023 22:35
Assert.Equal(originalParentSpanId, activity.ParentSpanId);
#else
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this silently failing up until now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so? I tried to look back at the run where it was introduced to see how it got in initially but detailed logging isn't maintained that far back.

The actual code always sets parentSpanId to default so I'm not sure how it ever ended up something other than what the root has.

@utpilla utpilla merged commit 18e54f3 into open-telemetry:main Dec 13, 2023
97 of 98 checks passed
@CodeBlanch CodeBlanch deleted the ci-solution-build-failures branch December 13, 2023 00:39
@Kielek Kielek added the infra Infra work - CI/CD, code coverage, linters label Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Infra work - CI/CD, code coverage, linters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants