-
Notifications
You must be signed in to change notification settings - Fork 186
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
OTLP/grpc #230
OTLP/grpc #230
Conversation
Signed-off-by:Ritick Gautam <riticksinghrajput@gmail.com>
updated .gitignore
updated script file updated Makefile
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.
A couple questions to start!
|
||
list($response, $status) = $client->Export($request)->wait(); | ||
if ($status->code !== Grpc\STATUS_OK) { | ||
echo "ERROR: " . $status->code . ", " . $status->details . PHP_EOL; |
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.
Is there a reason this is an error and exit(1)
rather than a return value?
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.
Not any specifc reason for error , just it will get exit if the status is not equal to STATUS_OK
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.
But the exit(1)
will completely shutdown the execution of the application, where the library is installed. Is this a desired behaviour?
@riticksingh - what else needs to be completed (besides the conflict resolution) for this PR? |
@jodeev - I believe @riticksingh is still working on this and it's not ready for review yet. I'm going to move it back to in progress. |
@riticksingh - are you still working on this PR? |
Hey folks, I was interested in the PR and have had a play around. I've got a working PoC of otel-php sending spans to the otel-collector and to Honeycomb.io over GRPC. Certainly not ready for prime time, but I'll get the commits up on a branch shortly. |
Sounds great, would love to see it @SeanHood ! |
|
||
declare(strict_types=1); | ||
namespace OpenTelemetry\Contrib\OtlpGrpc; | ||
require __DIR__ . '/../../vendor/autoload.php'; |
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.
Is there a reason to require the autoload here? I think it should stay only in the entry-point of an application.
<?php | ||
|
||
declare(strict_types=1); | ||
require __DIR__ . '/../vendor/autoload.php'; |
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.
same case as above.
As promised: SeanHood@cf54582 The commit has more info of what still needs sorting (that I can see). Sample from what I get in the otel collector:
|
No description provided.