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

implement rule-based sampler #279

Merged
merged 15 commits into from
Sep 12, 2024
Merged

Conversation

brettmc
Copy link
Collaborator

@brettmc brettmc commented Jul 4, 2024

Ported rule-based sampler from https://github.com/Nevay/otel-sdk-contrib-sampler

Co-authored-by: Tobias Bachert [email protected]

Copy link

codecov bot commented Jul 4, 2024

Codecov Report

Attention: Patch coverage is 32.14286% with 114 lines in your changes missing coverage. Please review.

Project coverage is 82.68%. Comparing base (727236e) to head (75c26b6).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...leBased/src/ComponentProvider/SamplerRuleBased.php 0.00% 28 Missing ⚠️
...sed/src/ComponentProvider/SamplingRuleSpanKind.php 0.00% 26 Missing ⚠️
...ed/src/ComponentProvider/SamplingRuleAttribute.php 0.00% 20 Missing ⚠️
...sed/src/ComponentProvider/SamplingRuleSpanName.php 0.00% 14 Missing ⚠️
...leBased/src/ComponentProvider/SamplingRuleLink.php 0.00% 13 Missing ⚠️
...Based/src/ComponentProvider/SamplingRuleParent.php 0.00% 13 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #279      +/-   ##
============================================
+ Coverage     82.45%   82.68%   +0.23%     
+ Complexity     1073      948     -125     
============================================
  Files           104       89      -15     
  Lines          4480     3807     -673     
============================================
- Hits           3694     3148     -546     
+ Misses          786      659     -127     
Flag Coverage Δ
Aws 85.75% <ø> (?)
Aws:7.4 ?
Aws:8.0 ?
Aws:8.1 ?
Aws:8.2 ?
Aws:8.3 ?
Context/Swoole 0.00% <ø> (?)
Context/Swoole:7.4 ?
Context/Swoole:8.0 ?
Context/Swoole:8.1 ?
Context/Swoole:8.2 ?
Context/Swoole:8.3 ?
Instrumentation/CakePHP:8.0 ?
Instrumentation/CakePHP:8.1 ?
Instrumentation/CakePHP:8.2 ?
Instrumentation/CakePHP:8.3 ?
Instrumentation/CodeIgniter 73.94% <ø> (?)
Instrumentation/CodeIgniter:8.0 ?
Instrumentation/CodeIgniter:8.1 ?
Instrumentation/CodeIgniter:8.2 ?
Instrumentation/CodeIgniter:8.3 ?
Instrumentation/ExtAmqp 89.58% <ø> (?)
Instrumentation/ExtAmqp:8.2 ?
Instrumentation/ExtAmqp:8.3 ?
Instrumentation/Guzzle 69.73% <ø> (?)
Instrumentation/Guzzle:8.0 ?
Instrumentation/Guzzle:8.1 ?
Instrumentation/Guzzle:8.2 ?
Instrumentation/Guzzle:8.3 ?
Instrumentation/HttpAsyncClient 81.33% <ø> (?)
Instrumentation/HttpAsyncClient:8.0 ?
Instrumentation/HttpAsyncClient:8.1 ?
Instrumentation/HttpAsyncClient:8.2 ?
Instrumentation/HttpAsyncClient:8.3 ?
Instrumentation/IO 70.90% <ø> (?)
Instrumentation/IO:8.2 ?
Instrumentation/IO:8.3 ?
Instrumentation/Laravel:8.0 ?
Instrumentation/Laravel:8.1 ?
Instrumentation/Laravel:8.2 ?
Instrumentation/Laravel:8.3 ?
Instrumentation/MongoDB 77.33% <ø> (?)
Instrumentation/MongoDB:7.4 ?
Instrumentation/MongoDB:8.0 ?
Instrumentation/MongoDB:8.1 ?
Instrumentation/MongoDB:8.2 ?
Instrumentation/MongoDB:8.3 ?
Instrumentation/OpenAIPHP 86.82% <ø> (?)
Instrumentation/OpenAIPHP:8.1 ?
Instrumentation/OpenAIPHP:8.2 ?
Instrumentation/OpenAIPHP:8.3 ?
Instrumentation/PDO 89.56% <ø> (?)
Instrumentation/PDO:8.2 ?
Instrumentation/PDO:8.3 ?
Instrumentation/Psr14 78.12% <ø> (?)
Instrumentation/Psr14:8.0 ?
Instrumentation/Psr14:8.1 ?
Instrumentation/Psr14:8.2 ?
Instrumentation/Psr14:8.3 ?
Instrumentation/Psr15 93.50% <ø> (?)
Instrumentation/Psr15:8.0 ?
Instrumentation/Psr15:8.1 ?
Instrumentation/Psr15:8.2 ?
Instrumentation/Psr15:8.3 ?
Instrumentation/Psr16:8.0 ?
Instrumentation/Psr16:8.1 ?
Instrumentation/Psr16:8.2 ?
Instrumentation/Psr16:8.3 ?
Instrumentation/Psr18 82.08% <ø> (?)
Instrumentation/Psr18:8.0 ?
Instrumentation/Psr18:8.1 ?
Instrumentation/Psr18:8.2 ?
Instrumentation/Psr18:8.3 ?
Instrumentation/Psr3:8.0 ?
Instrumentation/Psr3:8.1 ?
Instrumentation/Psr3:8.2 ?
Instrumentation/Psr3:8.3 ?
Instrumentation/Psr6:8.0 ?
Instrumentation/Psr6:8.1 ?
Instrumentation/Psr6:8.2 ?
Instrumentation/Psr6:8.3 ?
Instrumentation/Slim 86.95% <ø> (?)
Instrumentation/Slim:8.0 ?
Instrumentation/Slim:8.1 ?
Instrumentation/Slim:8.2 ?
Instrumentation/Slim:8.3 ?
Instrumentation/Symfony 89.03% <ø> (?)
Instrumentation/Symfony:8.0 ?
Instrumentation/Symfony:8.1 ?
Instrumentation/Symfony:8.2 ?
Instrumentation/Symfony:8.3 ?
Instrumentation/Yii 77.77% <ø> (?)
Instrumentation/Yii:8.0 ?
Instrumentation/Yii:8.1 ?
Instrumentation/Yii:8.2 ?
Instrumentation/Yii:8.3 ?
Logs/Monolog 100.00% <ø> (?)
Logs/Monolog:7.4 ?
Logs/Monolog:8.0 ?
Logs/Monolog:8.1 ?
Logs/Monolog:8.2 ?
Logs/Monolog:8.3 ?
Propagation/ServerTiming 100.00% <ø> (?)
Propagation/ServerTiming:8.0 ?
Propagation/ServerTiming:8.1 ?
Propagation/ServerTiming:8.2 ?
Propagation/ServerTiming:8.3 ?
Propagation/TraceResponse 100.00% <ø> (?)
Propagation/TraceResponse:7.4 ?
Propagation/TraceResponse:8.0 ?
Propagation/TraceResponse:8.1 ?
Propagation/TraceResponse:8.2 ?
Propagation/TraceResponse:8.3 ?
ResourceDetectors/Azure:7.4 ?
ResourceDetectors/Azure:8.0 ?
ResourceDetectors/Azure:8.1 ?
ResourceDetectors/Azure:8.2 ?
ResourceDetectors/Azure:8.3 ?
ResourceDetectors/Container 93.02% <ø> (?)
ResourceDetectors/Container:8.0 ?
ResourceDetectors/Container:8.1 ?
ResourceDetectors/Container:8.2 ?
ResourceDetectors/Container:8.3 ?
Sampler/RuleBased 32.14% <32.14%> (?)
Shims/OpenTracing 92.99% <ø> (?)
Shims/OpenTracing:7.4 ?
Shims/OpenTracing:8.0 ?
Shims/OpenTracing:8.1 ?
Shims/OpenTracing:8.2 ?
Shims/OpenTracing:8.3 ?
Symfony 88.20% <ø> (?)
Symfony:7.4 ?
Symfony:8.0 ?
Symfony:8.1 ?
Symfony:8.2 ?
Symfony:8.3 ?

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

Files with missing lines Coverage Δ
src/Sampler/RuleBased/src/RuleBasedSampler.php 100.00% <100.00%> (ø)
src/Sampler/RuleBased/src/RuleSet.php 100.00% <100.00%> (ø)
...mpler/RuleBased/src/SamplingRule/AttributeRule.php 100.00% <100.00%> (ø)
...rc/Sampler/RuleBased/src/SamplingRule/LinkRule.php 100.00% <100.00%> (ø)
.../Sampler/RuleBased/src/SamplingRule/ParentRule.php 100.00% <100.00%> (ø)
...ampler/RuleBased/src/SamplingRule/SpanKindRule.php 100.00% <100.00%> (ø)
...ampler/RuleBased/src/SamplingRule/SpanNameRule.php 100.00% <100.00%> (ø)
...leBased/src/ComponentProvider/SamplingRuleLink.php 0.00% <0.00%> (ø)
...Based/src/ComponentProvider/SamplingRuleParent.php 0.00% <0.00%> (ø)
...sed/src/ComponentProvider/SamplingRuleSpanName.php 0.00% <0.00%> (ø)
... and 3 more

... and 33 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

to avoid future collision with an official rule-based sampler, change the name to something less likely to conflict.
@brettmc brettmc marked this pull request as ready for review July 18, 2024 07:10
@brettmc brettmc requested a review from a team July 18, 2024 07:10
@brettmc
Copy link
Collaborator Author

brettmc commented Jul 18, 2024

I think this is ready for review now, @open-telemetry/php-approvers
There's an OTEP which looks like it will eventually specify a rules-based sampler, so to avoid a future config collision I've changed the key for this one to contrib_rule_based (open to improved names, though!)

@ChrisLightfootWild ChrisLightfootWild requested a review from a team August 29, 2024 22:23
Copy link
Contributor

@ChrisLightfootWild ChrisLightfootWild left a comment

Choose a reason for hiding this comment

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

Apologies for the delay 🐌

I've been reading this as a reference point for the ComponentProvider configuration.

Maybe we could add .gitattributes as well to reduce package size, but happy for that to be a follow up.

src/Sampler/RuleBased/src/RuleSetInterface.php Outdated Show resolved Hide resolved
@brettmc brettmc merged commit e5a13ce into open-telemetry:main Sep 12, 2024
96 of 120 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants