KEMBAR78
feat: Add OpenTelemetry Traces to GRPC by sydney-munro · Pull Request #2783 · googleapis/java-storage · GitHub
Skip to content

Conversation

@sydney-munro
Copy link
Contributor

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.

@sydney-munro sydney-munro requested a review from frankyn October 18, 2024 18:35
@sydney-munro sydney-munro requested a review from a team as a code owner October 18, 2024 18:35
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/java-storage API. labels Oct 18, 2024
@sydney-munro sydney-munro requested a review from a team as a code owner October 22, 2024 15:27
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • .github/sync-repo-settings.yaml

/**
* Enable OpenTelemetry Tracing and provide an instance for the client to use.
*
* @param openTelemetrySdk
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need a description of the param here, or just delete the @param line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. added a description.

@sydney-munro sydney-munro merged commit 5e6eb84 into otel-v1-branch Oct 22, 2024
14 checks passed
@sydney-munro sydney-munro deleted the otel-grpc branch October 22, 2024 16:34
// NoOp for Grpc
StorageOptions storageOptionsGrpc = StorageOptions.grpc().build();
Storage storageGrpc = storageOptionsGrpc.getService();
storageGrpc.create(BucketInfo.of(grpcBucket));
Copy link
Contributor

Choose a reason for hiding this comment

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

What is no-op testing and checking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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'
Copy link
Contributor

Choose a reason for hiding this comment

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

Does Kokoro need to be updated internally to watch this branch as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ill double check!

String bucket = randomBucketName();
storage.create(BucketInfo.of(bucket));
TestExporter testExported = (TestExporter) exporter;
SpanData spanData = testExported.getExportedSpans().get(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if i understand this question. what do you mean by which spans are collected

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good start to assert span count; and expand from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the googleapis/java-storage API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants