Skip to content

Add alternative instrumentation abstraction#822

Merged
brettmc merged 3 commits into
open-telemetry:mainfrom
Nevay:feature/instrumentation
Oct 16, 2022
Merged

Add alternative instrumentation abstraction#822
brettmc merged 3 commits into
open-telemetry:mainfrom
Nevay:feature/instrumentation

Conversation

@Nevay

@Nevay Nevay commented Sep 10, 2022

Copy link
Copy Markdown
Contributor

Adds global (context-scoped) tracer provider & co, intended to replace the current InstrumentationInterface.

$scope = InstrumentationConfigurator::create()
    ->withTracerProvider($tracerProvider)
    ->withPropagator($propagator)
    ->activate();

try {
    // ...
} finally {
    $scope->detach();
}
$instrumentation = new Instrumentation('io.opentelemetry.contrib.php');
// ...
$span = $instrumentation
    ->tracer() // returns a tracer for the currently active tracer provider
    ->spanBuilder('test')
    ->startSpan()

@codecov

codecov Bot commented Sep 10, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #822 (d7c308d) into main (f73a835) will increase coverage by 0.22%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #822      +/-   ##
============================================
+ Coverage     81.52%   81.75%   +0.22%     
- Complexity     1925     1947      +22     
============================================
  Files           239      243       +4     
  Lines          5008     5069      +61     
============================================
+ Hits           4083     4144      +61     
  Misses          925      925
Flag Coverage Δ
7.4 81.23% <90.16%> (+0.10%) ⬆️
8.0 81.75% <95.08%> (+0.16%) ⬆️
8.1 81.89% <95.08%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...I/Common/Instrumentation/CachedInstrumentation.php 100.00% <100.00%> (ø)
src/API/Common/Instrumentation/Configurator.php 100.00% <100.00%> (ø)
src/API/Common/Instrumentation/ContextKeys.php 100.00% <100.00%> (ø)
src/API/Common/Instrumentation/Globals.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f73a835...d7c308d. Read the comment docs.

@brettmc brettmc mentioned this pull request Sep 12, 2022
Closed
Nevay
Nevay commented Sep 18, 2022
View reviewed changes

@Nevay Nevay left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we merge this: SpanProcessors and MetricReaders should run exports with the Context they were created in.

Comment thread deptrac.baseline.yaml Outdated
Comment thread src/API/Common/Instrumentation/InstrumentationConfigurator.php Outdated
use OpenTelemetry\Context\ScopeInterface;
use Psr\Log\LoggerInterface;

final class InstrumentationConfigurator

@Nevay Nevay Sep 18, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should this implement ImplicitContextKeyedInterface?

@Nevay
Nevay marked this pull request as ready for review September 18, 2022 12:06
@Nevay
Nevay requested a review from a team September 18, 2022 12:06
@brettmc

brettmc commented Oct 3, 2022

Copy link
Copy Markdown
Contributor

@Nevay happy for this to be merged?

@Nevay

Nevay commented Oct 9, 2022

Copy link
Copy Markdown
Contributor Author

Should be ready after a rebase.

@Nevay
Nevay force-pushed the feature/instrumentation branch from cb9149e to d7c308d Compare October 15, 2022 13:14
@brettmc
brettmc merged commit f6671dd into open-telemetry:main Oct 16, 2022
@Nevay Nevay mentioned this pull request Dec 1, 2022
Closed
@Nevay Nevay mentioned this pull request Dec 8, 2022
Closed
@Nevay
Nevay deleted the feature/instrumentation branch April 19, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettmc brettmc brettmc approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Nevay @brettmc