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

KIND::CONSUMER instrumentation implemented #307

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
100 changes: 100 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,103 @@
# Contributing to OpenTelemetry PHP Contrib

## Introduction

Thank you for your interest in contributing to OpenTelemetry PHP Contrib! This guide will help you set up your development environment, understand the contribution workflow, and troubleshoot common issues.

## Prerequisites

Before you begin, ensure you have the following installed:

- PHP (Supported versions: 8.1, 8.2, 8.3)
- [Composer](https://getcomposer.org/)
- [Docker](https://docs.docker.com/engine/install/)
- [Docker Compose](https://docs.docker.com/compose/install/)
- GNU Make (for running development commands)

## Workflow

1. Fork the repository and create a new branch.
2. Make your changes and commit them with clear messages.
3. Run `make all-checks` before submitting a PR to ensure your changes pass all checks.
4. Submit a pull request with a clear description.

### Setting Up the Development Environment

1. Clone the repository:
```sh
git clone https://github.com/open-telemetry/opentelemetry-php-contrib.git
cd opentelemetry-php-contrib
```

2. Copy the environment file:
```sh
cp .env.dist .env
```

3. Install dependencies:
```sh
make install
```

## Testing

To run all tests, execute:
```sh
make test
```

If you need to test against a specific PHP version:
```sh
PHP_VERSION=8.1 make test
```

## Common Issues & Troubleshooting

### 1. PHP and Dependency Management Issues

**Error:** Composer dependency resolution failure due to missing `composer.lock` file.

**Solution:** Generate a new `composer.lock` file by running:
```sh
composer update --lock
```
Then, reinstall dependencies:
```sh
composer install
```

### 2. PHPUnit Tests Not Running

**Error:** Running tests results in zero tests executed.

**Solution:** Ensure PHPUnit is correctly configured by checking `phpunit.xml`. If tests are not found, update the test directory paths in `phpunit.xml`. Then, retry running the tests.


## Code Style & Static Analysis

We use PHP-CS-Fixer, Psalm, and PHPStan for code quality checks. Before committing, run:
```sh
make style
make psalm
make phpstan
```

## API Documentation

To generate API documentation:
```sh
make phpdoc
```

## Further Help

- Join our discussions on the CNCF Slack in the [`#opentelemetry-php`](https://cloud-native.slack.com/archives/C01NFPCV44V) channel.
- Check the [public meeting notes](https://docs.google.com/document/d/1i1E4-_y4uJ083lCutKGDhkpi3n4_e774SBLi9hPLocw/edit) for past updates.

Thank you for contributing to OpenTelemetry PHP Contrib



Maintainers ([@open-telemetry/php-maintainers](https://github.com/orgs/open-telemetry/teams/php-maintainers)):

- [Bob Strecansky](https://github.com/bobstrecansky), Mailchimp
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
},
"require-dev": {
"composer/xdebug-handler": "^2.0",
"open-telemetry/dev-tools": "dev-main"
"open-telemetry/dev-tools": "dev-main",
"phpunit/phpunit": "^8.0 || ^9.5 || ^10.0"
},
"autoload": {
"psr-4": {
Expand Down
97 changes: 76 additions & 21 deletions src/Instrumentation/Symfony/src/MessengerInstrumentation.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ public static function register(): void
{
$instrumentation = new CachedInstrumentation('io.opentelemetry.contrib.php.symfony_messenger');

/**
* MessageBusInterface dispatches messages to the handlers.
*/
hook(

// Instrument MessageBusInterface (message dispatching)
hook(
MessageBusInterface::class,
'dispatch',
pre: static function (
Expand All @@ -54,19 +53,17 @@ public static function register(): void
$message = $params[0];
$messageClass = \get_class($message);

/** @psalm-suppress ArgumentTypeCoercion */
// Instrument dispatch as a "send" operation with SpanKind::KIND_PRODUCER
$builder = $instrumentation
->tracer()
->spanBuilder(\sprintf('DISPATCH %s', $messageClass))
->setSpanKind(SpanKind::KIND_PRODUCER)
->spanBuilder(\sprintf('publish %s', $messageClass))
Copy link

Choose a reason for hiding this comment

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

Why not "dispatch"?

Choose a reason for hiding this comment

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

The span types and names should be consistent with the spec here:
https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/#span-kind

But it's kind of confusing TBH, it mentions both "publish" and "send", in the example it gives publish shop.orders, but then it doesn't give publish in operation type or span kind. 🤯

@brettmc can you clarify the spec wording here maybe?

->setSpanKind(SpanKind::KIND_PRODUCER) // Set KIND_PRODUCER for dispatch
->setAttribute(TraceAttributes::CODE_FUNCTION, $function)
->setAttribute(TraceAttributes::CODE_NAMESPACE, $class)
->setAttribute(TraceAttributes::CODE_FILEPATH, $filename)
->setAttribute(TraceAttributes::CODE_LINENO, $lineno)

->setAttribute(self::ATTRIBUTE_MESSENGER_BUS, $class)
->setAttribute(self::ATTRIBUTE_MESSENGER_MESSAGE, $messageClass)
;
->setAttribute(self::ATTRIBUTE_MESSENGER_MESSAGE, $messageClass);

$parent = Context::getCurrent();
$span = $builder
Expand Down Expand Up @@ -103,9 +100,7 @@ public static function register(): void
}
);

/**
* SenderInterface sends messages to a transport.
*/
// Instrument SenderInterface (sending messages to transport)
hook(
SenderInterface::class,
'send',
Expand All @@ -121,28 +116,88 @@ public static function register(): void
$envelope = $params[0];
$messageClass = \get_class($envelope->getMessage());

/** @psalm-suppress ArgumentTypeCoercion */
// Instrument sending as a "send" operation with SpanKind::KIND_PRODUCER
$builder = $instrumentation
->tracer()
->spanBuilder(\sprintf('send %s', $messageClass))
->setSpanKind(SpanKind::KIND_PRODUCER) // Set KIND_PRODUCER for sending
->setAttribute(TraceAttributes::CODE_FUNCTION, $function)
->setAttribute(TraceAttributes::CODE_NAMESPACE, $class)
->setAttribute(TraceAttributes::CODE_FILEPATH, $filename)
->setAttribute(TraceAttributes::CODE_LINENO, $lineno)
->setAttribute(self::ATTRIBUTE_MESSENGER_TRANSPORT, $class)
->setAttribute(self::ATTRIBUTE_MESSENGER_MESSAGE, $messageClass);

$parent = Context::getCurrent();
$span = $builder
->setParent($parent)
->startSpan();

$context = $span->storeInContext($parent);

Context::storage()->attach($context);

return $params;
},
post: static function (
SenderInterface $sender,
array $params,
?Envelope $result,
?\Throwable $exception
): void {
$scope = Context::storage()->scope();
if (null === $scope) {
return;
}

$scope->detach();
$span = Span::fromContext($scope->context());

if (null !== $exception) {
$span->recordException($exception, [
TraceAttributes::EXCEPTION_ESCAPED => true,
]);
$span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage());
}

$span->end();
}
);

// Instrument the receiving of messages (consumer-side)
hook(
ReceiverInterface::class,
'get',
static function (
SenderInterface $bus,
array $params,
string $class,
string $function,
?string $filename,
?int $lineno,
) use ($instrumentation): array {
/** @var Envelope $envelope */
$envelope = $params[0];
$messageClass = \get_class($envelope->getMessage());

// Instrument receiving as a "consume" operation with SpanKind::KIND_CONSUMER
$builder = $instrumentation
->tracer()
->spanBuilder(\sprintf('SEND %s', $messageClass))
->setSpanKind(SpanKind::KIND_PRODUCER)
->spanBuilder(\sprintf('consume %s', $messageClass))
->setSpanKind(SpanKind::KIND_CONSUMER) // Set KIND_CONSUMER for receiving
->setAttribute(TraceAttributes::CODE_FUNCTION, $function)
->setAttribute(TraceAttributes::CODE_NAMESPACE, $class)
->setAttribute(TraceAttributes::CODE_FILEPATH, $filename)
->setAttribute(TraceAttributes::CODE_LINENO, $lineno)

->setAttribute(self::ATTRIBUTE_MESSENGER_TRANSPORT, $class)
->setAttribute(self::ATTRIBUTE_MESSENGER_MESSAGE, $messageClass)
;
->setAttribute(self::ATTRIBUTE_MESSENGER_MESSAGE, $messageClass);

$parent = Context::getCurrent();

$span = $builder
->setParent($parent)
->startSpan();

$context = $span->storeInContext($parent);

Context::storage()->attach($context);

return $params;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\Messenger\Transport\InMemory\InMemoryTransport;
use Symfony\Component\Messenger\Transport\InMemoryTransport as LegacyInMemoryTransport;
use Symfony\Component\Messenger\Transport\TransportInterface;
use Symfony\Component\Messenger\Transport\Receiver\ReceiverInterface;

final class SendEmailMessage
{
Expand All @@ -35,14 +35,26 @@ protected function getMessenger(): MessageBusInterface
{
return new MessageBus();
}

protected function getTransport()
{
// Symfony 6+
// Symfony 6+ version of the transport
if (class_exists(InMemoryTransport::class)) {
return new InMemoryTransport();
}

// Symfony 5+
// Symfony 5+ fallback
return new LegacyInMemoryTransport();
}

protected function getReceiver()
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this different enough from getTransport to require a method? It doesn't return a receiver but a transport...

Copy link
Contributor

Choose a reason for hiding this comment

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

@RichardChukwu have you had any time to look further into this?

Copy link
Contributor Author

@RichardChukwu RichardChukwu Oct 30, 2024

Choose a reason for hiding this comment

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

Hey @ChrisLightfootWild, thanks for asking.

Not really, I'll appreciate any assistance please

{
// Symfony 6+ version of the receiver
if (class_exists(ReceiverInterface::class)) {
return new InMemoryTransport(); // Example transport acting as a receiver
}

// Symfony 5+ fallback
return new LegacyInMemoryTransport();
}

Expand All @@ -56,8 +68,7 @@ protected function getTransport()
public function test_dispatch_message($message, string $spanName, int $kind, array $attributes)
{
$bus = $this->getMessenger();

$bus->dispatch($message);
$bus->dispatch($message); // Target the correct interface (MessageBusInterface)

$this->assertCount(1, $this->storage);

Expand All @@ -73,6 +84,36 @@ public function test_dispatch_message($message, string $spanName, int $kind, arr
}
}

/**
* Test consumer span when processing a message
*/
public function test_consume_message()
{
$transport = $this->getReceiver(); // Use the correct receiver interface
$message = new SendEmailMessage('Hello Consumer');
$envelope = new Envelope($message);

// Simulate receiving the message via ReceiverInterface::get
$transport->get();

// Simulate message consumption (processing)
$bus = $this->getMessenger();
$bus->dispatch($message);

// After message is consumed, we expect a consumer span
$this->assertCount(1, $this->storage);

/** @var ImmutableSpan $span */
$span = $this->storage[0];

// We expect this to be a consumer span
$this->assertEquals('CONSUME OpenTelemetry\Tests\Instrumentation\Symfony\tests\Integration\SendEmailMessage', $span->getName());
$this->assertEquals(SpanKind::KIND_CONSUMER, $span->getKind());

$this->assertTrue($span->getAttributes()->has(MessengerInstrumentation::ATTRIBUTE_MESSENGER_MESSAGE));
$this->assertEquals('OpenTelemetry\Tests\Instrumentation\Symfony\tests\Integration\SendEmailMessage', $span->getAttributes()->get(MessengerInstrumentation::ATTRIBUTE_MESSENGER_MESSAGE));
}

/**
* @dataProvider sendDataProvider
* @param mixed $message
Expand Down Expand Up @@ -172,7 +213,7 @@ public function dispatchDataProvider(): array
[
new SendEmailMessage('Hello Again'),
'DISPATCH OpenTelemetry\Tests\Instrumentation\Symfony\tests\Integration\SendEmailMessage',
SpanKind::KIND_PRODUCER,
SpanKind::KIND_PROCESS, // Correct SpanKind for dispatching
[
MessengerInstrumentation::ATTRIBUTE_MESSENGER_BUS => 'Symfony\Component\Messenger\MessageBus',
MessengerInstrumentation::ATTRIBUTE_MESSENGER_MESSAGE => 'OpenTelemetry\Tests\Instrumentation\Symfony\tests\Integration\SendEmailMessage',
Expand Down
Loading