-
Notifications
You must be signed in to change notification settings - Fork 215
feat(sdk): add custom sampler registry support #1867
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
base: main
Are you sure you want to change the base?
feat(sdk): add custom sampler registry support #1867
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1867 +/- ##
============================================
+ Coverage 68.03% 68.10% +0.06%
- Complexity 3012 3019 +7
============================================
Files 453 460 +7
Lines 8819 8850 +31
============================================
+ Hits 6000 6027 +27
- Misses 2819 2823 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
1e9d1f7 to
fd15d7f
Compare
Add ability to register custom sampler factories via the Registry, allowing users to extend the SDK with custom sampling strategies. Implementation: - Add SamplerFactoryInterface for sampler factory contracts - Add factory classes for all built-in samplers (AlwaysOn, AlwaysOff, TraceIdRatioBased, ParentBased variants) - Add Registry::registerSamplerFactory() and Registry::samplerFactory() - Refactor SamplerFactory to use the registry instead of hardcoded logic - Add _register.php to auto-register built-in sampler factories Tests: - Add unit tests for all sampler factory classes - Add registry tests for sampler factory registration - Add tests for custom sampler registration with clobber behavior
fd15d7f to
82be09b
Compare
Add reportUnmatchedIgnoredErrors: false to PHPStan config to prevent failures when ignore patterns for PHP 8.1-specific Symfony Config errors don't match on newer PHP versions.
Add tests/Integration/Config and tests/Unit/Config to the ignore paths for Symfony Config NodeParentInterface errors on PHP 8.1.
| #[\Override] | ||
| public function create(): SamplerInterface | ||
| { | ||
| $ratio = Configuration::getRatio(Variables::OTEL_TRACES_SAMPLER_ARG); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $ratio = Configuration::getRatio(Variables::OTEL_TRACES_SAMPLER_ARG); | |
| $ratio = Configuration::getRatio(Variables::OTEL_TRACES_SAMPLER_ARG, 1.); |
| #[\Override] | ||
| public function create(): SamplerInterface | ||
| { | ||
| $ratio = Configuration::getRatio(Variables::OTEL_TRACES_SAMPLER_ARG); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $ratio = Configuration::getRatio(Variables::OTEL_TRACES_SAMPLER_ARG); | |
| $ratio = Configuration::getRatio(Variables::OTEL_TRACES_SAMPLER_ARG, 1.); |
|
|
||
| use OpenTelemetry\SDK\Trace\SamplerInterface; | ||
|
|
||
| interface SamplerFactoryInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if we use EnvComponentLoader instead of adding another {type}Factory to work towards #1681 / to align with file-based config ComponentProviders.
If we want to start with just samplers:
class SamplerFactory
{
public function create(): SamplerInterface
{
$registry = new EnvComponentLoaderRegistry();
foreach (ServiceLoader::load(EnvComponentLoader::class) as $loader) {
$registry->register($loader);
}
$env = new EnvResolver();
$context = new Context();
$samplerName = $env->string(Variables::OTEL_TRACES_SAMPLER) ?? Defaults::OTEL_TRACES_SAMPLER;
return $registry->load(SamplerInterface::class, $samplerName, $env, $context);
}
}/**
* @implements EnvComponentLoader<SamplerInterface>
*/
final class SamplerLoaderTraceIdRatioBased implements EnvComponentLoader
{
#[\Override]
public function load(EnvResolver $env, EnvComponentLoaderRegistry $registry, Context $context): SamplerInterface
{
return new TraceIdRatioBasedSampler($env->numeric(Variables::OTEL_TRACES_SAMPLER_ARG, max: 1) ?? 1.);
}
#[\Override]
public function name(): string
{
return 'traceidratio';
}
}# _register.php
ServiceLoader::register(EnvComponentLoader::class, SamplerLoaderTraceIdRatioBased::class);
// ...
Summary
Add ability to register custom sampler factories via the Registry, allowing users to extend the SDK with custom sampling strategies.
Packages can now call
registerSamplerFactoryto register a factory for their custom, named sampler.SDK will now register its own samplers via autoloading as well:
Changes
Implementation
SamplerFactoryInterface- Contract for sampler factoriesAlwaysOnSamplerFactory,AlwaysOffSamplerFactory- Simple sampler factoriesTraceIdRatioBasedSamplerFactory- Ratio-based sampler factory (reads from env)ParentBasedAlwaysOnSamplerFactory,ParentBasedAlwaysOffSamplerFactory,ParentBasedTraceIdRatioSamplerFactory- Parent-based variantsRegistry::registerSamplerFactory()andRegistry::samplerFactory()- Registry methods_register.php- Auto-registers built-in sampler factoriesSamplerFactoryto use the registry instead of hardcoded logic