feat: Add OpenTelemetry Traces to GRPC #2783
Conversation
product-auto-label
bot
added
size: m
|
Warning: This pull request is touching the following templated files:
|
| /** | ||
| * Enable OpenTelemetry Tracing and provide an instance for the client to use. | ||
| * | ||
| * @param openTelemetrySdk |
There was a problem hiding this comment.
I think we need a description of the param here, or just delete the @param line
There was a problem hiding this comment.
done. added a description.
| // NoOp for Grpc | ||
| StorageOptions storageOptionsGrpc = StorageOptions.grpc().build(); | ||
| Storage storageGrpc = storageOptionsGrpc.getService(); | ||
| storageGrpc.create(BucketInfo.of(grpcBucket)); |
There was a problem hiding this comment.
What is no-op testing and checking?
There was a problem hiding this comment.
just checking to make sure that it still runs without issue and does nothing. basically just that it doesnt error out.
| - clirr | ||
| - units (8) | ||
| - units (11) | ||
| - 'Kokoro - Test: Integration' |
There was a problem hiding this comment.
Does Kokoro need to be updated internally to watch this branch as well?
There was a problem hiding this comment.
ill double check!
| String bucket = randomBucketName(); | ||
| storage.create(BucketInfo.of(bucket)); | ||
| TestExporter testExported = (TestExporter) exporter; | ||
| SpanData spanData = testExported.getExportedSpans().get(0); |
There was a problem hiding this comment.
Do you plan on testing which spans are collected per operation in addition to Span attributes? I think this would be more useful for Upload and Download ops.
There was a problem hiding this comment.
I don't know if i understand this question. what do you mean by which spans are collected
There was a problem hiding this comment.
Oh i meant; let's say you're uploading a large object to GCS. I would assume there's a span for CreateResumableUpload, then span for each WriteObject request. Wondering if there's a way to test that using spanData.
There was a problem hiding this comment.
ah I see, I haven't expanded this for that case yet but yeah we would want to test each span is generated. Maybe we could check for the size to make sure if we expect 3 writeObjectRequests we would have 3 spans
There was a problem hiding this comment.
It's a good start to assert span count; and expand from there.
To keep this more closely aligned with the introduction in HTTP the scope here is small.
To follow will be to add the attributes for recording exceptions and testing that flow.