-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/otel optimisation #5
Conversation
…exists on instrumentation boot
…lemetry into feature/otel-optimisation
* @param \OpenTelemetry\SDK\Trace\SpanProcessorInterface $spanProcessor | ||
* @param \OpenTelemetry\SDK\Resource\ResourceInfo $resource | ||
* @param \OpenTelemetry\SDK\Common\Attribute\AttributesBuilderInterface $attributesBuilder | ||
* @param array $links |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think you could specify an array type
} | ||
|
||
/** | ||
* @return \OpenTelemetry\SDK\Trace\EventInterface[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docblock is not consistent with dockblock in the defining of the $events property
return $pExportTraceServiceRequest; | ||
} | ||
|
||
protected function convertResourceSpans(ResourceInfo $resource): ResourceSpans |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dockblock is missing
return $pResourceSpans; | ||
} | ||
|
||
protected function convertScopeSpans(InstrumentationScopeInterface $instrumentationScope): ScopeSpans |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and one more
|
||
/** | ||
* @param Resource_|Span|Event|Link|InstrumentationScope $pElement | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return void
$pElement->setDroppedAttributesCount($attributes->getDroppedAttributesCount()); | ||
} | ||
|
||
protected function convertSpanKind(int $kind): int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dockblock is missing
}; | ||
} | ||
|
||
protected function convertStatusCode(string $status): int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docblock is missing
return StatusCode::STATUS_CODE_UNSET; | ||
} | ||
|
||
protected function convertSpan(SpanDataInterface $span): Span |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docblock is missing
return $pSpan; | ||
} | ||
|
||
protected static function traceFlags(SpanContextInterface $spanContext): int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docblock is missing
{ | ||
/** | ||
* @param string $spanId | ||
* @return void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @return void | |
* | |
* @return void |
$this->scheduledDelayNanos = $scheduledDelayMillis * 1_000_000; | ||
} | ||
|
||
protected function setMaxExportBatchSize(int $maxExportBatchSize, int $maxQueueSize): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docblock is missing
if ($cli) { | ||
[$vendor, $bin, $application] = explode('/', $cli[0]); | ||
|
||
if ($application === 'console') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it is good idea to strings such as argv
and console
to consts
/** | ||
* @return bool | ||
*/ | ||
public function isEnabled(): bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it is a part of interface - pls skip this comment, otherwise this method is not necessary
PR Description
Add a meaningful description here that will let us know what you want to fix with this PR or what functionality you want to add.
Steps before you submit a PR
contribution-license-agreement.txt
file with the following content:I hereby agree to Spryker\'s Contribution License Agreement in https://github.com/spryker/opentelemetry/blob/HASH_OF_COMMIT_YOU_ARE_BASING_YOUR_BRANCH_FROM_MASTER_BRANCH/CONTRIBUTING.md.
This is a mandatory step to make sure you are aware of the license agreement and agree to it.
HASH_OF_COMMIT_YOU_ARE_BASING_YOUR_BRANCH_FROM_MASTER_BRANCH
is a hash of the commit you are basing your branch from the master branch. You can take it from commits list of master branch before you submit a PR.Checklist