fix(otelzap): write core.ctx only when init by okhowang · Pull Request #7368 · open-telemetry/opentelemetry-go-contrib (original) (raw)

@okhowang

@linux-foundation-easycla

CLA Signed

The committers listed above are authorized under a signed CLA.

pellared

Choose a reason for hiding this comment

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

Can you please add a changelog entry?

@codecov

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.6%. Comparing base (b2aa398) to head (efbaed5).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@ Coverage Diff @@ ## main #7368 +/- ##

Coverage 81.6% 81.6%

Files 205 205
Lines 18186 18187 +1

Files with missing lines Coverage Δ
bridges/otelzap/core.go 97.9% <100.0%> (+<0.1%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

khushijain21

Choose a reason for hiding this comment

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

Thanks for catching this!

@dmathieu

@okhowang

Could this be tested?

it can be found by go test -race, and adding a test to call Write with context.Context field in two goroutine.

@dmathieu

Sure, but the CI does run go test with -race and doesn't fail on the main branch.

@pellared

@okhowang

dmathieu

Choose a reason for hiding this comment

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

This looks good, we now only need a changelog entry 😸

khushijain21

pellared

@okhowang

pellared

dmathieu

@pellared

MrAlias

@dmathieu

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})