Skip to content

fix(otel): Missing addLink method and Fiber handling #2849

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.

Sign up for GitHub

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

Merged
PROFeNoM merged 10 commits into from
Sep 17, 2024
Merged

fix(otel): Missing addLink method and Fiber handling #2849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1904119
fix(otel): Missing method and Fiber handling
PROFeNoM Sep 12, 2024
2ce47be
tests: `test_opentelemetry_{1|beta}`
PROFeNoM Sep 13, 2024
c50c2f0
tests: Properly set composer to 1.0.*
PROFeNoM Sep 13, 2024
ae66da9
fix: remove `cleanup_opentelemetry`
PROFeNoM Sep 13, 2024
22a2913
fix: php 7.4 unpacking issue
PROFeNoM Sep 13, 2024
1be1c34
tests: skip fiber test on coverage
PROFeNoM Sep 13, 2024
f69790f
style: remove useless require
PROFeNoM Sep 13, 2024
705a254
remove comment
PROFeNoM Sep 13, 2024
1c14c3b
perf: condition check
PROFeNoM Sep 16, 2024
e44f4e4
perf: Class lookup optimization
PROFeNoM Sep 16, 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
22 changes: 21 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,7 @@ TEST_INTEGRATIONS_81 := \
test_integrations_mysqli \
test_integrations_openai \
test_opentelemetry_1 \
test_opentelemetry_beta \
test_integrations_guzzle7 \
test_integrations_pcntl \
test_integrations_pdo \
Expand Down Expand Up @@ -898,6 +899,7 @@ TEST_INTEGRATIONS_82 := \
test_integrations_mysqli \
test_integrations_openai \
test_opentelemetry_1 \
test_opentelemetry_beta \
test_integrations_guzzle7 \
test_integrations_pcntl \
test_integrations_pdo \
Expand Down Expand Up @@ -958,6 +960,7 @@ TEST_INTEGRATIONS_83 := \
test_integrations_mysqli \
test_integrations_openai \
test_opentelemetry_1 \
test_opentelemetry_beta \
test_integrations_guzzle7 \
test_integrations_pcntl \
test_integrations_pdo \
Expand Down Expand Up @@ -1133,10 +1136,27 @@ benchmarks: benchmarks_run_dependencies call_benchmarks

benchmarks_opcache: benchmarks_run_dependencies call_benchmarks_opcache

test_opentelemetry_1: global_test_run_dependencies tests/Frameworks/Custom/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) tests/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR)
define setup_opentelemetry
cp $(1) $(dir $(1))/composer.json
endef

define run_opentelemetry_tests
$(eval TEST_EXTRA_ENV=$(shell [ $(PHP_MAJOR_MINOR) -ge 81 ] && echo "OTEL_PHP_FIBERS_ENABLED=1" || echo '') DD_TRACE_OTEL_ENABLED=1 DD_TRACE_GENERATE_ROOT_SPAN=0)
$(call run_tests,--testsuite=opentelemetry1 $(TESTS))
$(eval TEST_EXTRA_ENV=)
endef

_test_opentelemetry_beta_setup: global_test_run_dependencies
$(call setup_opentelemetry,tests/OpenTelemetry/composer-beta.json)

test_opentelemetry_beta: _test_opentelemetry_beta_setup tests/Frameworks/Custom/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) tests/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR)
$(call run_opentelemetry_tests)

_test_opentelemetry_1_setup: global_test_run_dependencies
$(call setup_opentelemetry,tests/OpenTelemetry/composer-1.json)

test_opentelemetry_1: _test_opentelemetry_1_setup tests/Frameworks/Custom/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) tests/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR)
$(call run_opentelemetry_tests)

test_opentracing_10: global_test_run_dependencies tests/OpenTracer1Unit/composer.lock-php$(PHP_MAJOR_MINOR) tests/Frameworks/Custom/OpenTracing/composer.lock-php$(PHP_MAJOR_MINOR)
$(call run_tests,tests/OpenTracer1Unit)
Expand Down
12 changes: 10 additions & 2 deletions src/DDTrace/OpenTelemetry/Context.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ final class Context implements ContextInterface
/** @var ContextStorageInterface&ExecutionContextAwareInterface */
private static ContextStorageInterface $storage;

/** @var string $storageClass */
private static string $storageClass = '';

// Optimization for spans to avoid copying the context array.
private static ContextKeyInterface $spanContextKey;
private ?object $span = null;
Expand Down Expand Up @@ -58,8 +61,13 @@ public static function setStorage(ContextStorageInterface $storage): void
*/
public static function storage(): ContextStorageInterface
{
/** @psalm-suppress RedundantPropertyInitializationCheck */
return self::$storage ??= new ContextStorage();
if (self::$storageClass === '') {
self::$storageClass = class_exists('OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC')
? 'OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC' // v1.1+
: 'OpenTelemetry\Context\ContextStorage';
}

return self::$storage ??= new self::$storageClass();
}

/**
Expand Down
93 changes: 65 additions & 28 deletions src/DDTrace/OpenTelemetry/Span.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,28 +112,18 @@ private function __construct(
foreach ($links as $link) {
/** @var LinkInterface $link */
$linkContext = $link->getSpanContext();

$spanLink = new SpanLink();
$spanLink->traceId = $linkContext->getTraceId();
$spanLink->spanId = $linkContext->getSpanId();
$spanLink->traceState = (string)$linkContext->getTraceState(); // __toString()
$spanLink->attributes = $link->getAttributes()->toArray();
$spanLink->droppedAttributesCount = 0; // Attributes limit aren't supported/meaningful in DD

// Save the link
ObjectKVStore::put($spanLink, "link", $link);
$span->links[] = $spanLink;
$span->links[] = $this->createAndSaveSpanLink($linkContext, $link->getAttributes()->toArray(), $link);
}

foreach ($events as $event) {
/** @var EventInterface $event */

$spanEvent = new SpanEvent(
$event->getName(),
$event->getName(),
$event->getAttributes()->toArray(),
$event->getEpochNanos()
);

// Save the event
ObjectKVStore::put($spanEvent, "event", $event);
$span->events[] = $spanEvent;
Expand Down Expand Up @@ -235,17 +225,33 @@ public function toSpanData(): SpanDataInterface
$this->updateSpanLinks();
$this->updateSpanEvents();

return new ImmutableSpan(
$this,
$this->getName(),
$this->links,
$this->events,
Attributes::create(array_merge($this->span->meta, $this->span->metrics)),
0,
StatusData::create($this->status->getCode(), $this->status->getDescription()),
$hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0,
$this->hasEnded()
);
if (method_exists(SpanInterface::class, 'addLink')) {
// v1.1 backward compatibility: totalRecordedLinks parameter added
return new ImmutableSpan(
$this,
$this->getName(),
$this->links,
$this->events,
Attributes::create(array_merge($this->span->meta, $this->span->metrics)),
$this->totalRecordedEvents,
$this->totalRecordedLinks,
StatusData::create($this->status->getCode(), $this->status->getDescription()),
$hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0,
$this->hasEnded(),
);
} else {
return new ImmutableSpan(
$this,
$this->getName(),
$this->links,
$this->events,
Attributes::create(array_merge($this->span->meta, $this->span->metrics)),
$this->totalRecordedEvents,
StatusData::create($this->status->getCode(), $this->status->getDescription()),
$hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0,
$this->hasEnded(),
);
}
}

/**
Expand Down Expand Up @@ -358,14 +364,27 @@ public function setAttributes(iterable $attributes): SpanInterface
return $this;
}

/**
* @inheritDoc
*/
public function addLink(SpanContextInterface $context, iterable $attributes = []): SpanInterface
{
if ($this->hasEnded() || !$context->isValid()) {
return $this;
}

$this->span->links[] = $this->createAndSaveSpanLink($context, $attributes);
return $this;
}

/**
* @inheritDoc
*/
public function addEvent(string $name, iterable $attributes = [], int $timestamp = null): SpanInterface
{
if (!$this->hasEnded()) {
$this->span->events[] = new SpanEvent(
$name,
$name,
$attributes,
$timestamp ?? (int)(microtime(true) * 1e9)
);
Expand Down Expand Up @@ -522,7 +541,7 @@ private function updateSpanEvents()
{
$datadogSpanEvents = $this->span->events;
$this->span->meta["events"] = count($this->events);

$otel = [];
foreach ($datadogSpanEvents as $datadogSpanEvent) {
$exceptionAttributes = [];
Expand All @@ -539,17 +558,35 @@ private function updateSpanEvents()
$event = new Event(
$datadogSpanEvent->name,
(int)$datadogSpanEvent->timestamp,
Attributes::create(array_merge($exceptionAttributes, \is_array($datadogSpanEvent->attributes) ? $datadogSpanEvent->attributes : iterator_to_array($datadogSpanEvent->attributes)))
Attributes::create(array_merge($exceptionAttributes, \is_array($datadogSpanEvent->attributes) ? $datadogSpanEvent->attributes : iterator_to_array($datadogSpanEvent->attributes)))
);

// Save the event
ObjectKVStore::put($datadogSpanEvent, "event", $event);
}
$otel[] = $event;
}

// Update the events
$this->events = $otel;
$this->totalRecordedEvents = count($otel);
}

private function createAndSaveSpanLink(SpanContextInterface $context, iterable $attributes = [], LinkInterface $link = null)
{
$spanLink = new SpanLink();
$spanLink->traceId = $context->getTraceId();
$spanLink->spanId = $context->getSpanId();
$spanLink->traceState = (string)$context->getTraceState(); // __toString()
$spanLink->attributes = $attributes;
$spanLink->droppedAttributesCount = 0; // Attributes limit aren't supported/meaningful in DD

// Save the link
if (is_null($link)) {
$link = new Link($context, Attributes::create($attributes));
}
ObjectKVStore::put($spanLink, "link", $link);

return $spanLink;
}
}
74 changes: 1 addition & 73 deletions ...lemetry/Integration/Context/FiberTest.php → ...y/Integration/Context/Fiber/FiberTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use OpenTelemetry\API\Trace\Span;
use OpenTelemetry\API\Trace\SpanKind;
use OpenTelemetry\Context\Context;
use OpenTelemetry\Context\ContextStorage;
use OpenTelemetry\Context\ExecutionContextAwareInterface;
use OpenTelemetry\SDK\Trace\TracerProvider;
use function DDTrace\close_span;
use function DDTrace\start_span;
Expand All @@ -31,78 +31,6 @@ public function ddSetUp(): void
public function ddTearDown()
{
parent::ddTearDown();
Context::setStorage(new ContextStorage()); // Reset OpenTelemetry context
}

/**
* @throws \Throwable
*/
public function test_context_switching_ffi_observer()
{
$key = Context::createKey('-');
$scope = Context::getCurrent()
->with($key, 'main')
->activate();

$fiber = new Fiber(function () use ($key) {
$scope = Context::getCurrent()
->with($key, 'fiber')
->activate();

$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));

Fiber::suspend();

$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));

$scope->detach();
});

$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));

$fiber->start();

$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));

$fiber->resume();

$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));

$scope->detach();
}

public function test_context_switching_ffi_observer_registered_on_startup()
{
$key = Context::createKey('-');

$fiber = new Fiber(function () use ($key) {
$scope = Context::getCurrent()
->with($key, 'fiber')
->activate();

$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));

Fiber::suspend();

$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));

$scope->detach();
});


$fiber->start();

$this->assertSame('main:', 'main:' . Context::getCurrent()->get($key));

$scope = Context::getCurrent()
->with($key, 'main')
->activate();

$fiber->resume();

$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));

$scope->detach();
}

public function testFiberInteroperabilityStackSwitch()
Expand Down
46 changes: 46 additions & 0 deletions tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
--TEST--
Context switches on execution context switch.
--SKIPIF--
<?php if (PHP_VERSION_ID < 80100 || !extension_loaded('ffi') || getenv('PHPUNIT_COVERAGE')) die('skip requires PHP8.1 and FFI'); ?>
--ENV--
OTEL_PHP_FIBERS_ENABLED=1
--FILE--
<?php
use OpenTelemetry\Context\Context;

require_once './tests/OpenTelemetry/vendor/autoload.php';

$key = Context::createKey('-');
$scope = Context::getCurrent()
->with($key, 'main')
->activate();

$fiber = new Fiber(function() use ($key) {
$scope = Context::getCurrent()
->with($key, 'fiber')
->activate();

echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL;

Fiber::suspend();
echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL;

$scope->detach();
});

echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;

$fiber->start();
echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;

$fiber->resume();
echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;

$scope->detach();
?>
--EXPECT--
main:main
fiber:fiber
main:main
fiber:fiber
main:main
Loading