Skip to content

Conversation

cartermp
Copy link
Contributor

Related to #2098

We had a team using otel-go see the integer values of span status codes in honeycomb, then went to look them up on pkg.go.dev. However, they got very confused because the values of Ok and Error are flipped w.r.t. OTLP. I think calling this out in the comments, which also enables seeing the note in editor tools, can reduce future confusion on this topic.

I think that this could also go in the docs, but not exclusively there because people will still use editor tooling and pkg.go.dev to look things up.

@codecov
Copy link

codecov bot commented Oct 21, 2022

Codecov Report

Merging #3386 (a9e1984) into main (797e337) will increase coverage by 0.0%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #3386   +/-   ##
=====================================
  Coverage   77.9%   77.9%           
=====================================
  Files        164     164           
  Lines      11361   11361           
=====================================
+ Hits        8857    8859    +2     
+ Misses      2305    2303    -2     
  Partials     199     199           
Impacted Files Coverage Δ
codes/codes.go 90.4% <ø> (ø)
sdk/trace/batch_span_processor.go 81.1% <0.0%> (+0.8%) ⬆️

@dmathieu dmathieu added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 21, 2022
@dmathieu
Copy link
Member

This is a nit, but I wonder if we shouldn't write this in a more generic format than just OTLP. AFAIK, other libraries could also be using different numbers. Something like:

// NOTE: The value of this enum is only relevant to the internals
// of the Go SDK. Other implementations may use different values.
// For example, OTLP uses 2.

@cartermp
Copy link
Contributor Author

Yeah, a more generic message could work too.

@MrAlias MrAlias merged commit 3b9862a into open-telemetry:main Oct 28, 2022
@MrAlias MrAlias added this to the Release v1.11.2 milestone Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants