Skip to content

ext-mongodb 2.0 support #324

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
merged 3 commits into from
Jan 14, 2025
Merged

ext-mongodb 2.0 support #324

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8f5af3
mongodb 2.0 support
brettmc Jan 7, 2025
483e0bf
Merge branch 'main' into mongodb-2
brettmc Jan 14, 2025
fbb65ce
document mongo todos
brettmc Jan 14, 2025
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
8 changes: 7 additions & 1 deletion docker-compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ services:
PHP_IDE_CONFIG: ${PHP_IDE_CONFIG:-''}
RABBIT_HOST: ${RABBIT_HOST:-rabbitmq}
KAFKA_HOST: ${KAFKA_HOST:-kafka}
MONGODB_HOST: ${MONGODB_HOST:-mongodb}
MONGODB_PORT: ${MONGODB_PORT:-27017}

zipkin:
image: openzipkin/zipkin-slim
Expand Down Expand Up @@ -60,5 +62,9 @@ services:
command: "bash -c '/tmp/update_run.sh && /etc/confluent/docker/run'"
volumes:
- ./docker/kafka/update_run.sh:/tmp/update_run.sh

mongodb:
image: mongo:4
hostname: mongodb
ports:
- "27017:27017/tcp"

6 changes: 3 additions & 3 deletions src/Instrumentation/MongoDB/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
"homepage": "https://opentelemetry.io/docs/php",
"readme": "./README.md",
"license": "Apache-2.0",
"minimum-stability": "dev",
"require": {
"php": ">=7.4",
"ext-mongodb": "*",
"ext-mongodb": "^1.13",
"ext-json": "*",
"mongodb/mongodb": "^1.15",
"open-telemetry/api": "^1.0",
Expand Down Expand Up @@ -40,7 +39,8 @@
},
"config": {
"allow-plugins": {
"php-http/discovery": false
"php-http/discovery": false,
"tbachert/spi": false
}
}
}
5 changes: 5 additions & 0 deletions src/Instrumentation/MongoDB/phpstan.neon.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,8 @@ parameters:
paths:
- src
- tests
ignoreErrors:
-
message: "#Call to an undefined method .*#"
paths:
- src/MongoDBInstrumentationSubscriber.php
99 changes: 85 additions & 14 deletions src/Instrumentation/MongoDB/src/MongoDBInstrumentationSubscriber.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@
use MongoDB\Driver\Monitoring\CommandStartedEvent;
use MongoDB\Driver\Monitoring\CommandSubscriber;
use MongoDB\Driver\Monitoring\CommandSucceededEvent;
use MongoDB\Driver\Monitoring\SDAMSubscriber;
use MongoDB\Driver\Monitoring\ServerChangedEvent;
use MongoDB\Driver\Monitoring\ServerClosedEvent;
use MongoDB\Driver\Monitoring\ServerHeartbeatFailedEvent;
use MongoDB\Driver\Monitoring\ServerHeartbeatStartedEvent;
use MongoDB\Driver\Monitoring\ServerHeartbeatSucceededEvent;
use MongoDB\Driver\Monitoring\ServerOpeningEvent;
use MongoDB\Driver\Monitoring\TopologyChangedEvent;
use MongoDB\Driver\Monitoring\TopologyClosedEvent;
use MongoDB\Driver\Monitoring\TopologyOpeningEvent;
use OpenTelemetry\API\Instrumentation\CachedInstrumentation;
use OpenTelemetry\API\Trace\Span;
use OpenTelemetry\API\Trace\SpanBuilderInterface;
Expand All @@ -18,13 +28,17 @@
use OpenTelemetry\SemConv\TraceAttributes;
use Throwable;

final class MongoDBInstrumentationSubscriber implements CommandSubscriber
final class MongoDBInstrumentationSubscriber implements CommandSubscriber, SDAMSubscriber
{
private CachedInstrumentation $instrumentation;
/**
* @var Closure(object):?string
*/
private Closure $commandSerializer;
/**
* @var array<string, array<int, array<string, mixed>>>
*/
private array $serverAttributes = [];

/**
* @param (callable(object):?string) $commandSerializer
Expand All @@ -41,16 +55,26 @@ public function __construct(CachedInstrumentation $instrumentation, callable $co
};
}

/**
* @psalm-suppress MixedAssignment,MixedArrayTypeCoercion,MixedArrayOffset,MixedArgument
*/
public function commandStarted(CommandStartedEvent $event): void
{
$command = $event->getCommand();
$collectionName = MongoDBCollectionExtractor::extract($command);
$databaseName = $event->getDatabaseName();
$commandName = $event->getCommandName();
$server = $event->getServer();
$info = $server->getInfo();
$port = $server->getPort();
$host = $server->getHost();
/** @phpstan-ignore-next-line */
if (version_compare(phpversion('mongodb'), '1.20.0', '>=')) {
$host = $event->getHost();
$port = $event->getPort();
} else {
$server = $event->getServer();
$host = $server->getHost();
$port = $server->getPort();
}
$attributes = $this->serverAttributes[$host][$port] ?? [];

$isSocket = str_starts_with($host, '/');
/** @psalm-suppress RiskyTruthyFalsyComparison **/
$scopedCommand = ($collectionName ? $collectionName . '.' : '') . $commandName;
Expand All @@ -65,17 +89,10 @@ public function commandStarted(CommandStartedEvent $event): void
->setAttribute(TraceAttributes::NETWORK_TRANSPORT, $isSocket ? 'unix' : 'tcp')
->setAttribute(TraceAttributes::DB_STATEMENT, ($this->commandSerializer)($command))
->setAttribute(TraceAttributes::DB_MONGODB_COLLECTION, $collectionName)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_MASTER, $info['ismaster'] ?? null)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_READ_ONLY, $info['readOnly'] ?? null)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_CONNECTION_ID, $info['connectionId'] ?? null)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_REQUEST_ID, $event->getRequestId())
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_OPERATION_ID, $event->getOperationId())
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_MAX_WIRE_VERSION, $info['maxWireVersion'] ?? null)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_MIN_WIRE_VERSION, $info['minWireVersion'] ?? null)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_MAX_BSON_OBJECT_SIZE_BYTES, $info['maxBsonObjectSize'] ?? null)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_MAX_MESSAGE_SIZE_BYTES, $info['maxMessageSizeBytes'] ?? null)
->setAttribute(MongoDBTraceAttributes::DB_MONGODB_MAX_WRITE_BATCH_SIZE, $info['maxWriteBatchSize'] ?? null);

->setAttributes($attributes)
;
$parent = Context::getCurrent();
$span = $builder->startSpan();
Context::storage()->attach($span->storeInContext($parent));
Expand Down Expand Up @@ -118,4 +135,58 @@ private static function endSpan(?Throwable $exception = null): void

$span->end();
}

public function serverChanged(ServerChangedEvent $event): void
{
$host = $event->getHost();
$port = $event->getPort();
$info = $event->getNewDescription()->getHelloResponse();
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a corner case mentioned in MongoDB extension documentation at https://www.php.net/manual/en/mongodb-driver-serverdescription.gethelloresponse.php

When the driver is connected to a load balancer, this method will return an empty array since load balancers are not monitored. This is in contrast to MongoDB\Driver\Server::getInfo(), which would return the backing server's » hello command response from the initial connection handshake.

The code comment regarding this still seems present in 2.x branch of mongo extension, so this is probably still the case even though the above documentation is for 1.x. I don't see an obvious workaround for 2.x, but possibly if this $info here is empty array and we detect that mongo extension version is 1.x, there could be a fallback to the previous behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know what they mean by "load balancer"? I set up a replicaSet and it correctly fetches info from all servers via the serverChanged event.

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked what the extension tests with for tests that run "with load balancer", and it seems they use haproxy as seen here. On the driver side it seems adding loadBalanced=true to connection URL parameters should make the driver behave as if it is connecting through a load balancer, so maybe no actual load balancer is required to trigger this behavior, just that parameter? The specification is here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ugh, loadBalanced=true seems to require a real, working, load-balanced setup, and I cannot get a load-balanced mongo cluster running.

I've got a 3-node replicaset, and haproxy in front. I can connect through haproxy and round-robin to the different nodes, however it's not "real" load balancing as adding loadBalanced=true gives me a Driver attempted to initialize in load balancing mode, but the server does not support this mode.

Since our mongo package is not stable (latest=0.0.6), the easiest path forward is to go ahead with this change, and wait for somebody with a load-balanced setup to show up (and/or create a todo/help-wanted issue).

Another point to consider is that all of these extra fields are not part of the official semconv, and according to https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/telemetry-stability.md#stable-instrumentations they should not be used in a stable instrumentation. It's not a problem now, but we may end up needing to remove them in the future anyway, if we're following the spec dogmatically:

Stable instrumentations authored by OpenTelemetry SHOULD NOT produce telemetry that is not described by OpenTelemetry semantic conventions

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems reasonable. 👍

Copy link

Choose a reason for hiding this comment

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

Regarding load balancers, some drivers do use HAPorxy for local testing but the actual feature is more involved and entails the backing servers also being aware of the load balancer (and returning a serviceId field in their hello responses). The Load Balancer specification goes into more detail about the feature.

$attributes = [
MongoDBTraceAttributes::DB_MONGODB_MASTER => $info['ismaster'] ?? null,
MongoDBTraceAttributes::DB_MONGODB_READ_ONLY => $info['readOnly'] ?? null,
MongoDBTraceAttributes::DB_MONGODB_CONNECTION_ID => $info['connectionId'] ?? null,
MongoDBTraceAttributes::DB_MONGODB_MAX_WIRE_VERSION => $info['maxWireVersion'] ?? null,
MongoDBTraceAttributes::DB_MONGODB_MIN_WIRE_VERSION => $info['minWireVersion'] ?? null,
MongoDBTraceAttributes::DB_MONGODB_MAX_BSON_OBJECT_SIZE_BYTES => $info['maxBsonObjectSize'] ?? null,
MongoDBTraceAttributes::DB_MONGODB_MAX_MESSAGE_SIZE_BYTES => $info['maxMessageSizeBytes'] ?? null,
MongoDBTraceAttributes::DB_MONGODB_MAX_WRITE_BATCH_SIZE => $info['maxWriteBatchSize'] ?? null,
];
$this->serverAttributes[$host][$port] = $attributes;
}

public function serverOpened(ServerOpeningEvent $event): void
{
}

public function serverClosed(ServerClosedEvent $event): void
{
}

public function serverOpening(ServerOpeningEvent $event): void
{
}

public function serverHeartbeatFailed(ServerHeartbeatFailedEvent $event): void
{
}

public function serverHeartbeatStarted(ServerHeartbeatStartedEvent $event): void
{
}

public function serverHeartbeatSucceeded(ServerHeartbeatSucceededEvent $event): void
{
}

public function topologyChanged(TopologyChangedEvent $event): void
{
}

public function topologyClosed(TopologyClosedEvent $event): void
{
}

public function topologyOpening(TopologyOpeningEvent $event): void
{
}
}
12 changes: 9 additions & 3 deletions src/Instrumentation/MongoDB/tests/Integration/MongoDBInstrumentationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,19 @@ class MongoDBInstrumentationTest extends TestCase
{
private const DATABASE_NAME = 'db';
private const COLLECTION_NAME = 'coll';
private string $host;
private int $port;
private string $uri;
private ScopeInterface $scope;
/** @var ArrayObject<int,ImmutableSpan> */
private ArrayObject $storage;
private ?ImmutableSpan $span = null;

public function setUp(): void
{
$this->host = $_SERVER['MONGODB_HOST'] ?? '127.0.0.1';
$this->port = (int) ($_SERVER['MONGODB_PORT'] ?? 27017);
$this->uri = "mongodb://$this->host:$this->port";
/** @psalm-suppress MixedPropertyTypeCoercion */
$this->storage = new ArrayObject();
$tracerProvider = new TracerProvider(
Expand All @@ -49,7 +55,7 @@ public function tearDown(): void

public function test_mongodb_find_one(): void
{
$manager = new Manager('mongodb://127.0.0.1:27017');
$manager = new Manager($this->uri);

$find = new FindOne(self::DATABASE_NAME, self::COLLECTION_NAME, ['a' => 'b']);

Expand All @@ -67,8 +73,8 @@ public function test_mongodb_find_one(): void
self::assertSame(self::DATABASE_NAME, $attributes->get(TraceAttributes::DB_NAME));
self::assertSame('find', $attributes->get(TraceAttributes::DB_OPERATION));
self::assertSame(self::COLLECTION_NAME, $attributes->get(TraceAttributes::DB_MONGODB_COLLECTION));
self::assertSame('127.0.0.1', $attributes->get(TraceAttributes::SERVER_ADDRESS));
self::assertSame(27017, $attributes->get(TraceAttributes::SERVER_PORT));
self::assertSame($this->host, $attributes->get(TraceAttributes::SERVER_ADDRESS));
self::assertSame($this->port, $attributes->get(TraceAttributes::SERVER_PORT));
self::assertSame('tcp', $attributes->get(TraceAttributes::NETWORK_TRANSPORT));
self::assertTrue($attributes->get(MongoDBTraceAttributes::DB_MONGODB_MASTER));
self::assertFalse($attributes->get(MongoDBTraceAttributes::DB_MONGODB_READ_ONLY));
Expand Down
Loading