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

Add OpenCensus bridge internal package #2146

Merged
merged 9 commits into from
Aug 6, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Jul 29, 2021

This move is to enable access to internal types in a planned test module (see #2144 for more information).

The only functional change is in the error handling. The global error handler cannot be reset multiple times (#2140) so a local variable is used get around this. Additionally, the conversion of the span start options now returns an error instead of trying to formulate a error with the context of the span it is being called for.

In addition to moving the implementation to an internal package this adds tests for all the conversions and methods.

Unit tests for the conversion functions and types methods will be added in a follow on PR.

@MrAlias MrAlias added pkg:bridges Related to a bridge package Skip Changelog PRs that do not require a CHANGELOG.md entry labels Jul 29, 2021
@MrAlias MrAlias mentioned this pull request Jul 29, 2021
5 tasks
@codecov
Copy link

codecov bot commented Jul 29, 2021

Codecov Report

Merging #2146 (aa4ca0f) into main (fcf945a) will decrease coverage by 0.0%.
The diff coverage is 85.4%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2146     +/-   ##
=======================================
- Coverage   72.2%   72.2%   -0.1%     
=======================================
  Files        170     176      +6     
  Lines      12076   12088     +12     
=======================================
+ Hits        8724    8731      +7     
- Misses      3111    3115      +4     
- Partials     241     242      +1     
Impacted Files Coverage Δ
bridge/opencensus/utils/utils.go 50.0% <50.0%> (-50.0%) ⬇️
...pencensus/internal/oc2otel/tracer_start_options.go 72.2% <72.2%> (ø)
bridge/opencensus/internal/tracer.go 77.2% <77.2%> (ø)
bridge/opencensus/internal/span.go 86.9% <86.9%> (ø)
bridge/opencensus/internal/oc2otel/attributes.go 90.4% <90.4%> (ø)
bridge/opencensus/bridge.go 100.0% <100.0%> (+14.8%) ⬆️
bridge/opencensus/internal/oc2otel/span_context.go 100.0% <100.0%> (ø)
bridge/opencensus/internal/otel2oc/span_context.go 100.0% <100.0%> (ø)
... and 4 more

@MrAlias MrAlias marked this pull request as draft July 29, 2021 18:28
@MrAlias
Copy link
Contributor Author

MrAlias commented Jul 29, 2021

I'm going to pull the addition of tests out of this and put it in a follow on PR to accelerate the review of this. This PR will not only block the addition of tests PR, but also blocks the refactor of the integration tests into a test module that uses the default SDK.

@MrAlias MrAlias marked this pull request as ready for review July 29, 2021 18:33
@MrAlias MrAlias merged commit d18c135 into open-telemetry:main Aug 6, 2021
@MrAlias MrAlias deleted the oc-internal branch August 6, 2021 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:bridges Related to a bridge package 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