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

Wrap errors returned from Detect and New in sdk/resource #3844

Merged
merged 5 commits into from
Mar 8, 2023

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Mar 7, 2023

Currently, the errors returned from these functions do not wrap errors. This means that external users cannot use the Go errors.Is function to test their error is successfully returned from their resource detectors1. This resolves this introducing an unwrap-able detectErr that is returned from an new internal copy of Detect.

The internal copy of Detect, detect, is used to allow New to pass a resource with the appropriate schema URL set. This means the schema URL collision of detect is reused instead of duplicating the functionality for New. Also, this avoid an additional allocation when the returned *Resource from Detect in New was discarded when it was later merged into another new *Resource.

Footnotes

  1. https://github.com/open-telemetry/opentelemetry-go/pull/3812#discussion_r1120995919

@MrAlias MrAlias added enhancement New feature or request area:resources Part of OpenTelemetry resources labels Mar 7, 2023
@MrAlias MrAlias added this to the v1.15.0 milestone Mar 7, 2023
@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #3844 (179bb20) into main (60f7d42) will increase coverage by 0.0%.
The diff coverage is 94.2%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #3844   +/-   ##
=====================================
  Coverage   81.7%   81.7%           
=====================================
  Files        168     168           
  Lines      12462   12477   +15     
=====================================
+ Hits       10182   10195   +13     
- Misses      2065    2067    +2     
  Partials     215     215           
Impacted Files Coverage Δ
sdk/resource/auto.go 90.6% <93.9%> (+0.6%) ⬆️
sdk/resource/resource.go 76.5% <100.0%> (-1.4%) ⬇️
sdk/trace/batch_span_processor.go 81.1% <0.0%> (-0.9%) ⬇️
exporters/jaeger/jaeger.go 91.1% <0.0%> (+0.8%) ⬆️

Test that New returns an error that can be unwrapped.
@MrAlias MrAlias merged commit 1626ff7 into open-telemetry:main Mar 8, 2023
@MrAlias MrAlias deleted the res-err-is branch March 8, 2023 23:15
@MrAlias MrAlias modified the milestones: v1.15.0, Metric v0.38.0 Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:resources Part of OpenTelemetry resources enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants