FRW-8618 Init commit #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gechetspr
requested changes
Aug 23, 2024
composer.json
Outdated
| "open-telemetry/context": "^1.0", | ||
| "open-telemetry/sem-conv": "^1.0", | ||
| "php": ">=8.1", | ||
| "spryker/rabbit-mq": "dev-master", |
| ->setAttribute(static::ATTRIBUTE_QUEUE_NAME, $params[0]); | ||
|
|
||
| if (static::isValidMessage($params)) { | ||
| $array = json_decode($params[1][0]->getBody(), true); |
There was a problem hiding this comment.
Please name variables in more descriptive name. Array is quite generic name
| ->startSpan() | ||
| ->activate(); | ||
| }, | ||
| post: function ($instance, array $params, $response, ?Throwable $exception): void { |
| $span->recordException($exception); | ||
| $span->setStatus(StatusCode::STATUS_ERROR); | ||
| } else { | ||
|
|
There was a problem hiding this comment.
please remove this empty line
| } | ||
|
|
||
| $endTime = microtime(true); | ||
| $duration = $endTime - $span->getAttribute(static::START_TIME); |
There was a problem hiding this comment.
After some consideration, not sure if needed.
First of all span has getDuration method.
And also it's auto added to the span
| */ | ||
| protected static function isValidMessage(array $params): bool | ||
| { | ||
| return array_key_exists(1, $params) |
There was a problem hiding this comment.
Suggested change
| return array_key_exists(1, $params) | |
| $PLEASE_RENAME_TO_SMTH_DESCRIPTIVE = $params[1][0] ?? null; | |
| if (!$PLEASE_RENAME_TO_SMTH_DESCRIPTIVE) { | |
| return false; | |
| } | |
| return $PLEASE_RENAME_TO_SMTH_DESCRIPTIVE instanceof QueueSendMessageTransfer && is_string($PLEASE_RENAME_TO_SMTH_DESCRIPTIVE->getBody()); |
gechetspr
reviewed
Aug 27, 2024
composer.json
Outdated
| "open-telemetry/context": "^1.0", | ||
| "open-telemetry/sem-conv": "^1.0", | ||
| "php": ">=8.1", | ||
| "spryker/rabbit-mq": "^2.19.0", |
There was a problem hiding this comment.
I would say we need a version, but we should not force people to update to the latest one. I would say ^2.0.0 is more then enough
gechetspr
approved these changes
Aug 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Developer(s): @dimitriyTsemma
Ticket: https://spryker.atlassian.net/browse/FRW-8630
merge: squash
Release Table
Package OtelRabbitMqInstrumentation
Change log
Initial Release