Skip to content
Closed
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
130 changes: 123 additions & 7 deletions src/Psr/Tracing/RelayOpenTelemetry.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ class RelayOpenTelemetry
'_prefix',
];

/**
* Maximum string length when formatting commands for OpenTelemetry.
*
* @var int
*/
protected const MAX_STR_LEN = 100;

/**
* Creates a new instance.
*
Expand Down Expand Up @@ -112,9 +119,13 @@ public function __call(string $name, array $arguments)
return $this->relay->{$name}(...$arguments);
}

$operation = strtoupper($name);
$stmt = $this->fmtCommand($operation, $arguments);

$span = $this->tracer->spanBuilder('Relay::' . strtolower($name))
->setAttribute('db.operation', strtoupper($name))
->setAttribute('db.system', 'redis')
->setAttribute('db.operation', $operation)
->setAttribute('db.statement', $stmt)
Comment thread
vmihailenco marked this conversation as resolved.
Comment on lines +127 to +128
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This won't work with __construct() and connect() and some other, which aren't Redis commands, but Relay specific methods.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"This" is db.statement? Why it does work for db.operation?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's also sending invalid data currently. We're working on the stubs to make this easier.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will make it easier to identify various types of methods: https://relay.so/api

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, sounds like I should wait for some input from you.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vmihailenco: We're exploring whether we can use 8.0 attributes in the class stubs. If so we could use reflection to check whether a method should be bypassed (setOption()), should be traced (connect()) or whether it's a Redis command (mget()).

Instead of maintaining arrays of functions we could parse all class methods once, or each method once when it's executed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for update. Let me know what changes do you expect here.

My 5 cents are that I would not worry about this. I'd say that tracing connect is the right thing to do and having setOption also would not hurt. Are you worried about having db.statement on such spans? Again, I don't see what real issues that causes.,,

->setSpanKind(SpanKind::KIND_CLIENT)
->startSpan();

Expand Down Expand Up @@ -152,9 +163,11 @@ public static function __callStatic(string $name, array $arguments)
*/
public function scan(&$iterator, $match = null, int $count = 0, ?string $type = null)
{
$stmt = $this->fmtScan('SCAN', null, $iterator, $match, $count, $type);
$span = $this->tracer->spanBuilder('Relay::scan')
->setAttribute('db.operation', 'SCAN')
->setAttribute('db.system', 'redis')
->setAttribute('db.operation', 'SCAN')
->setAttribute('db.statement', $stmt)
->setSpanKind(SpanKind::KIND_CLIENT)
->startSpan();

Expand All @@ -180,9 +193,11 @@ public function scan(&$iterator, $match = null, int $count = 0, ?string $type =
*/
public function hscan($key, &$iterator, $match = null, int $count = 0)
{
$stmt = $this->fmtScan('HSCAN', $key, $iterator, $match, $count);
$span = $this->tracer->spanBuilder('Relay::hscan')
->setAttribute('db.operation', 'HSCAN')
->setAttribute('db.system', 'redis')
->setAttribute('db.operation', 'HSCAN')
->setAttribute('db.statement', $stmt)
->setSpanKind(SpanKind::KIND_CLIENT)
->startSpan();

Expand All @@ -208,9 +223,11 @@ public function hscan($key, &$iterator, $match = null, int $count = 0)
*/
public function sscan($key, &$iterator, $match = null, int $count = 0)
{
$stmt = $this->fmtScan('SSCAN', $key, $iterator, $match, $count);
$span = $this->tracer->spanBuilder('Relay::sscan')
->setAttribute('db.operation', 'SSCAN')
->setAttribute('db.system', 'redis')
->setAttribute('db.operation', 'SSCAN')
->setAttribute('db.statement', $stmt)
->setSpanKind(SpanKind::KIND_CLIENT)
->startSpan();

Expand All @@ -236,9 +253,11 @@ public function sscan($key, &$iterator, $match = null, int $count = 0)
*/
public function zscan($key, &$iterator, $match = null, int $count = 0)
{
$stmt = $this->fmtScan('ZSCAN', $key, $iterator, $match, $count);
$span = $this->tracer->spanBuilder('Relay::zscan')
->setAttribute('db.operation', 'ZSCAN')
->setAttribute('db.system', 'redis')
->setAttribute('db.operation', 'ZSCAN')
->setAttribute('db.statement', $stmt)
->setSpanKind(SpanKind::KIND_CLIENT)
->startSpan();

Expand Down Expand Up @@ -298,9 +317,11 @@ public function executeBufferedTransaction(Transaction $transaction)
? 'pipeline'
: 'multi';

$span = $this->tracer->spanBuilder('Relay::exec')
->setAttribute('db.operation', 'EXEC')
$stmt = $this->fmtCommands($transaction->commands);
$span = $this->tracer->spanBuilder('Relay::' . $method)
->setAttribute('db.system', 'redis')
->setAttribute('db.operation', $method)
->setAttribute('db.statement', $stmt)
->setSpanKind(SpanKind::KIND_CLIENT)
->startSpan();

Expand All @@ -320,4 +341,99 @@ public function executeBufferedTransaction(Transaction $transaction)
$span->end();
}
}

/**
* Formats multiple commands for db.statement attribute.
*
* @param array<mixed> $commands
* @return string
*/
protected function fmtCommands(array $commands)
{
$acc = [];
$len = 0;
foreach ($commands as $command) {
$command = $this->fmtCommand($command[0], $command[1]);
$len += strlen($command);
if ($len > 5000) {
break;
}
array_push($acc, $command);
}
return implode(PHP_EOL, $acc);
}

/**
* Formats a single command for db.statement attribute.
*
* @param string $name
* @param array<mixed> $arguments
* @return string
*/
protected function fmtCommand(string $operation, array $arguments)
{
$acc = [$operation];
$len = 0;
$this->fmtArguments($acc, $len, $arguments);
return implode(' ', $acc);
}

/**
* Formats passed arguments as strings.
*
* @param array<string> $acc
* @param array<mixed> $arguments
* @return void
*/
protected function fmtArguments(&$acc, int &$len, $arguments)
{
foreach ($arguments as $value) {
if (is_array($value)) {
$this->fmtArguments($acc, $len, $value);
continue;
}

$str = strval($value);
if (strlen($str) > self::MAX_STR_LEN) {
$str = substr($str, 0, self::MAX_STR_LEN - 3) + '...';
}

$len += strlen($str);
if ($len > 1000) {
array_push($acc, '...');
break;
}
array_push($acc, $str);
}
}

/**
* Formats SCAN command arguments.
*
* @param string $name
* @param mixed $key
* @param mixed $iterator
* @param mixed $match
* @param int $count
* @param ?string $type
* @return string
*/
protected function fmtScan(string $name, $key, $iterator, $match = null, int $count = 0, ?string $type = null)
{
$args = [$name];
if ($key) {
array_push($args, $key);
}
array_push($args, $iterator);
if ($match) {
array_push($args, 'MATCH', $match);
}
if ($count > 0) {
array_push($args, 'COUNT', $count);
}
if ($type) {
array_push($args, 'TYPE', $type);
}
return implode(' ', $args);
}
}