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 scope configuration #1353

Merged
merged 39 commits into from
Aug 18, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
c12f89d
add enabled function
brettmc Jul 18, 2024
7f8324b
tracer configurator
brettmc Jul 18, 2024
b61feed
logger config
brettmc Jul 18, 2024
3111994
meter config
brettmc Jul 18, 2024
6248609
allow tracer reconfiguration after creation
brettmc Jul 19, 2024
1a4fca7
test enabling a tracer mid-trace
brettmc Jul 19, 2024
0d70f91
tracer configurator builder
brettmc Jul 19, 2024
8c6fb99
consolidate configurators and config
brettmc Jul 19, 2024
dbd1108
Merge branch 'main' into scope-config
brettmc Jul 20, 2024
776f87a
metrics reconfig
brettmc Jul 20, 2024
3c8cd17
gauge enabled
brettmc Jul 20, 2024
46c5730
rector
brettmc Jul 20, 2024
3bdae48
default values
brettmc Jul 20, 2024
8447c8d
instrument disabled if meter disabled
brettmc Jul 20, 2024
83f81c6
rename enabled to isEnabled
brettmc Jul 20, 2024
ccf7da5
do not export metrics when meter disabled
brettmc Jul 21, 2024
9b33ef7
add benchmark test
brettmc Jul 21, 2024
705bfc4
update logger config on provider config update
brettmc Jul 21, 2024
0feb995
tests
brettmc Jul 21, 2024
3955dba
rename method, add tests
brettmc Jul 22, 2024
0eb8707
rename method, tests
brettmc Jul 22, 2024
0447da6
add configurable interface to providers
brettmc Jul 22, 2024
471d391
interface
brettmc Jul 22, 2024
f94a733
ScopeConfigurator interface
brettmc Jul 22, 2024
4d21c6d
param
brettmc Jul 22, 2024
25c7249
remove duplicate interface implementation
brettmc Jul 22, 2024
f30df57
nullsafe, rename method
brettmc Jul 22, 2024
9807215
fix weakmap GC
brettmc Jul 23, 2024
48cf8fa
do not export metrics of disabled meters
brettmc Jul 23, 2024
4dcdb01
update benchmark per review feedback
brettmc Jul 23, 2024
65ff1d3
use wildcards instead of regex pattern in name predicate
brettmc Jul 23, 2024
6d018cc
rector + style
brettmc Jul 23, 2024
a233ad7
Merge branch 'main' into scope-config
brettmc Jul 23, 2024
d2c9793
add isEnabled to late binding tracer/logger/meter
brettmc Jul 23, 2024
b5f0c72
disable metrics streams instead of callbacks
brettmc Jul 24, 2024
6fb6b1d
add skipped test for disabling metric streams
brettmc Jul 24, 2024
e7e0104
note a todo on reconfiguring meters
brettmc Jul 25, 2024
1d78401
replace scope configurator implementation
brettmc Jul 25, 2024
b2f89b6
Merge branch 'main' into scope-config
brettmc Aug 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,7 @@ parameters:
message: "#Call to an undefined method Symfony\\\\Component\\\\Config\\\\Definition\\\\Builder\\\\NodeParentInterface::.*#"
paths:
- src/Config/SDK
-
message: "#Cannot call method .* on null#"
paths:
- tests/Integration/SDK/Trace
2 changes: 1 addition & 1 deletion src/API/Logs/LoggerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ public function emit(LogRecord $logRecord): void;
* are about to generate a LogRecord, to avoid performing computationally expensive work.
* @experimental
*/
public function enabled(): bool;
public function isEnabled(): bool;
}
2 changes: 1 addition & 1 deletion src/API/Logs/NoopLogger.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function log($level, $message, array $context = []): void
{
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/API/Metrics/GaugeInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*
* @experimental
*/
interface GaugeInterface
interface GaugeInterface extends SynchronousInstrument
{

/**
Expand Down
7 changes: 6 additions & 1 deletion src/API/Metrics/Instrument.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ interface Instrument
{
/**
* Determine if the instrument is enabled. Instrumentation authors SHOULD call this API each time they record a measurement.
*
* MUST return false if:
* - The MeterConfig of the Meter used to create the instrument has parameter disabled=true
* - All resolved views for the instrument are configured with the Drop Aggregation
* @experimental
* @see https://opentelemetry.io/docs/specs/otel/metrics/sdk/#instrument-enabled
*/
public function enabled(): bool;
public function isEnabled(): bool;
}
2 changes: 2 additions & 0 deletions src/API/Metrics/MeterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,6 @@ public function createObservableUpDownCounter(
array|callable $advisory = [],
callable ...$callbacks,
): ObservableUpDownCounterInterface;

public function isEnabled(): bool;
}
2 changes: 1 addition & 1 deletion src/API/Metrics/Noop/NoopCounter.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public function add($amount, iterable $attributes = [], $context = null): void
// no-op
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
5 changes: 5 additions & 0 deletions src/API/Metrics/Noop/NoopGauge.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,9 @@ public function record(float|int $amount, iterable $attributes = [], $context =
{
// no-op
}

public function isEnabled(): bool
{
return false;
}
}
2 changes: 1 addition & 1 deletion src/API/Metrics/Noop/NoopHistogram.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public function record($amount, iterable $attributes = [], $context = null): voi
// no-op
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
5 changes: 5 additions & 0 deletions src/API/Metrics/Noop/NoopMeter.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,9 @@ public function createObservableUpDownCounter(string $name, ?string $unit = null
{
return new NoopObservableUpDownCounter();
}

public function isEnabled(): bool
{
return false;
}
}
2 changes: 1 addition & 1 deletion src/API/Metrics/Noop/NoopObservableCounter.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function observe(callable $callback, bool $weaken = false): ObservableCal
return new NoopObservableCallback();
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/API/Metrics/Noop/NoopObservableGauge.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function observe(callable $callback, bool $weaken = false): ObservableCal
return new NoopObservableCallback();
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/API/Metrics/Noop/NoopObservableUpDownCounter.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function observe(callable $callback, bool $weaken = false): ObservableCal
return new NoopObservableCallback();
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/API/Metrics/Noop/NoopUpDownCounter.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public function add($amount, iterable $attributes = [], $context = null): void
// no-op
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/API/Trace/NoopTracer.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function spanBuilder(string $spanName): SpanBuilderInterface
return new NoopSpanBuilder(Context::storage());
}

public function enabled(): bool
public function isEnabled(): bool
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/API/Trace/TracerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ public function spanBuilder(string $spanName): SpanBuilderInterface;
* creating a new span.
* @experimental
*/
public function enabled(): bool;
public function isEnabled(): bool;
}
26 changes: 26 additions & 0 deletions src/SDK/Common/InstrumentationScope/Condition.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope;

use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;

class Condition implements ConditionInterface
{
public function __construct(
private readonly Predicate $predicate,
private readonly State $state,
) {
}

public function matches(InstrumentationScopeInterface $scope): bool
{
return $this->predicate->matches($scope);
}

public function state(): State
{
return $this->state;
}
}
13 changes: 13 additions & 0 deletions src/SDK/Common/InstrumentationScope/ConditionInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope;

use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;

interface ConditionInterface
{
public function matches(InstrumentationScopeInterface $scope): bool;
public function state(): State;
}
25 changes: 25 additions & 0 deletions src/SDK/Common/InstrumentationScope/Config.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope;

/**
* @internal
*/
class Config
{
public function __construct(private readonly State $state = State::ENABLED)
{
}

public function isEnabled(): bool
{
return $this->state === State::ENABLED;
}

public static function default(): self
{
return new self();
}
}
10 changes: 10 additions & 0 deletions src/SDK/Common/InstrumentationScope/Configurable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope;

interface Configurable
{
public function updateConfigurator(ScopeConfigurator $configurator): void;
}
41 changes: 41 additions & 0 deletions src/SDK/Common/InstrumentationScope/Configurator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope;

use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;

class Configurator implements ScopeConfigurator
{
/**
* @param list<Condition> $conditions
*/
public function __construct(private readonly array $conditions = [])
{
}

/**
* @internal
*/
public function getConfig(InstrumentationScopeInterface $scope): Config
{
foreach ($this->conditions as $condition) {
if ($condition->matches($scope)) {
return new Config($condition->state());
Copy link
Contributor

Choose a reason for hiding this comment

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

How will we add new configuration options if the spec is extended (esp. if signal specific options are added; do we want a dedicated config class per signal?)?


I'm currently playing around with an alternative approach that uses callbacks to modify the config rather than using the config state that is returned by the first matching condition; all matching callbacks are applied to the initial config, which would make supporting new config options trivial. The configurator uses a WeakMap<InstrumentationScope, TConfig> to update configs when a new callback is registered.

(new Configurator(static fn() => new MeterConfig()))
    ->with(static fn(MeterConfig $config) => $config->disabled = true, name: 'io.opentelemetry.contrib.*')
    ->with(static fn(MeterConfig $config) => $config->disabled = true, name: 'my-meter')

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that we do, or will, need a config class per signal. I was just trying to avoid copy-pasting the same code when they currently have the same functionality.
I don't mind addressing that sooner than later - either I do just turn the one general class into 3 signal-specific ones, or I'm happy to incorporate your alternative if you've got some more code to point me at?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was using the following Configurator implementation, with TracerProvider/MeterProvider/LoggerProvider accepting a Closure(InstrumentationScope): TConfig as configurator ("[Tracer/Meter/Logger]Configurator is modeled as a function to maximize flexibility.").

Configurator implementation

I'm not too happy with the state of this implementation as it does not support removal of callbacks yet.
Might want to add a predicate parameter to ::with() that allows flexible conditions.

/**
 * @template T
 */
final class Configurator {

    /** @var Closure(InstrumentationScope): T */
    private readonly Closure $factory;
    /** @var WeakMap<InstrumentationScope, T> */
    private WeakMap $configs;
    /** @var list<ConfiguratorClosure> */
    private array $configurators = [];

    /**
     * @param Closure(InstrumentationScope): T $factory
     */
    public function __construct(Closure $factory) {
        $this->configs = new WeakMap();
        $this->factory = $factory;
    }

    /**
     * @param Closure(T, InstrumentationScope): void $closure
     */
    public function with(Closure $closure, ?string $name, ?string $version = null, ?string $schemaUrl = null): self {
        $this->configurators[] = $configurator = new ConfiguratorClosure($closure, self::namePattern($name), $version, $schemaUrl);

        foreach ($this->configs as $instrumentationScope => $config) {
            if ($configurator->matches($instrumentationScope)) {
                ($configurator->closure)($config, $instrumentationScope);
            }
        }

        return $this;
    }

    /**
     * @return T
     */
    public function resolve(InstrumentationScope $instrumentationScope): object {
        if ($config = $this->configs[$instrumentationScope] ?? null) {
            return $config;
        }

        $config = ($this->factory)($instrumentationScope);
        foreach ($this->configurators as $configurator) {
            if ($configurator->matches($instrumentationScope)) {
                ($configurator->closure)($config, $instrumentationScope);
            }
        }

        return $this->configs[$instrumentationScope] ??= $config;
    }

    private static function namePattern(?string $name): ?string {
        return $name !== null
            ? sprintf('/^%s$/', strtr(preg_quote($name, '/'), ['\\?' => '.', '\\*' => '.*']))
            : null;
    }
}

/**
 * @internal
 */
final class ConfiguratorClosure {

    public function __construct(
        public readonly Closure $closure,
        private readonly ?string $name,
        private readonly ?string $version,
        private readonly ?string $schemaUrl,
    ) {}

    public function matches(InstrumentationScope $instrumentationScope): bool {
        return ($this->name === null || preg_match($this->name, $instrumentationScope->name))
            && ($this->version === null || $this->version === $instrumentationScope->version)
            && ($this->schemaUrl === null || $this->schemaUrl === $instrumentationScope->schemaUrl);
    }
}
$meterProviderBuilder->setMeterConfigurator((new Configurator(static fn() => new MeterConfig()))
    ->with(static fn(MeterConfig $config) => $config->setDisabled(true), name: '*')
    ->with(static fn(MeterConfig $config) => $config->setDisabled(false), name: 'test')
    ->resolve(...)):

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the Configurator implementation to use the above. I had to suppress a couple of phan/phpstan complaints about templates, but it works. I added a couple of convenience methods to create a default Configurator for each signal (Configurator::meter() etc).
I think it's also worth noting/documenting that if there are multiple matching rules in a configurator, then the last one wins.

}
}

return Config::default();
}

public static function builder(): ConfiguratorBuilder
{
return new ConfiguratorBuilder();
}

public static function default(): ScopeConfigurator
{
return new self();
}
}
23 changes: 23 additions & 0 deletions src/SDK/Common/InstrumentationScope/ConfiguratorBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope;

class ConfiguratorBuilder
{
/** @var list<Condition> */
private array $conditions = [];

public function addCondition(Predicate $predicate, State $state): ConfiguratorBuilder
{
$this->conditions[] = new Condition($predicate, $state);

return $this;
}

public function build(): Configurator
{
return new Configurator($this->conditions);
}
}
12 changes: 12 additions & 0 deletions src/SDK/Common/InstrumentationScope/Predicate.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope;

use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;

interface Predicate
{
public function matches(InstrumentationScopeInterface $scope): bool;
}
30 changes: 30 additions & 0 deletions src/SDK/Common/InstrumentationScope/Predicate/Attribute.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope\Predicate;

use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;
use OpenTelemetry\SDK\Common\InstrumentationScope\Predicate;

/**
* Predicate which matches exactly on key+value on an {@link InstrumentationScope} attribute.
*/
class Attribute implements Predicate
{
public function __construct(
private readonly string $key,
private readonly mixed $value,
) {
}

public function matches(InstrumentationScopeInterface $scope): bool
{
if (!$scope->getAttributes()->has($this->key)) {
return false;
}
$attribute = $scope->getAttributes()->get($this->key);

return $attribute === $this->value;
}
}
24 changes: 24 additions & 0 deletions src/SDK/Common/InstrumentationScope/Predicate/AttributeExists.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\SDK\Common\InstrumentationScope\Predicate;

use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;
use OpenTelemetry\SDK\Common\InstrumentationScope\Predicate;

/**
* Predicate which matches on the existence of an {@link InstrumentationScope} attribute.
*/
class AttributeExists implements Predicate
{
public function __construct(
private readonly string $key,
) {
}

public function matches(InstrumentationScopeInterface $scope): bool
{
return $scope->getAttributes()->has($this->key);
}
}
Loading
Loading