Skip to content
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

feat: add OpenTelemetry tracing #1568

Merged
merged 32 commits into from
Jun 27, 2024
Merged

feat: add OpenTelemetry tracing #1568

merged 32 commits into from
Jun 27, 2024

Conversation

olavloite
Copy link
Collaborator

Adds OpenTelemetry tracing to the JDBC driver. The OpenTelemetry instance that should be used for tracing should be either be set as the global OpenTelemetry instance, or set in the Properties map that is used to create a JDBC connection. In the latter case, the specific OpenTelemetry instance will only be used for that connection.

@olavloite olavloite requested review from a team as code owners April 18, 2024 15:59
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner-jdbc API. labels Apr 18, 2024
@googleapis googleapis deleted a comment from generated-files-bot bot May 30, 2024
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Jun 21, 2024
String instanceId = "my-instance";
String databaseId = "my-database";
// Setting this to true instructs the JDBC driver to include the SQL statement with the traces.
boolean enableExtendedTracing = true;

Choose a reason for hiding this comment

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

Should we also enableApiTracing in the code snipper

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added (although we need to wait for googleapis/java-spanner#3166 to be merged before we can use it here)

```

#### Example Using an OpenTelemetry instance in Properties
Create and register a global OpenTelemetry object before creating a JDBC connection.

Choose a reason for hiding this comment

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

We are not registering global OT object here.

nit: I feel we can simplify this example by not repeating the whole code .

We can create OpenTelemetrySdkBuilder object and then show different usage of it.

for eg.

 OpenTelemetrySdkBuilder openTelemetrySdkBuilder =
        OpenTelemetrySdk.builder()
            .setTracerProvider(
                SdkTracerProvider.builder()
                    .setSampler(Sampler.traceIdRatioBased(0.05))
                    .setResource(
                        Resource.builder()
                            .put("service.name", "my-unique-service-name")
                            .build())
                    .addSpanProcessor(BatchSpanProcessor.builder(traceExporter).build())
                    .build());
 // Example Using Global OpenTelemetry
 openTelemetrySdkBuilder.buildAndRegisterGlobal()
// Example Using an OpenTelemetry instance in Properties 
     OpenTelemetry openTelemetry = openTelemetrySdkBuilder.build()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for noting. Removed the text regarding registering a global OTEL object.

I also simplified the second example a bit, assuming that readers will understand that they can use the same OpenTelemetry instance as in the example above.

@@ -0,0 +1,257 @@
# Spanner JDBC Driver - Latency Debugging Guide

The Spanner JDBC driver supports OpenTelemetry tracing. Tracing can be used to investigate slow

Choose a reason for hiding this comment

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

We also have metrics which is helpful in debugging latency issues. Should that info also be added here https://cloud.google.com/spanner/docs/latency-points

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a link to the page. The intention is to also add this guide to the documentation (probably to the same submenu)

Choose a reason for hiding this comment

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

Yes, we currently don't have a very good observability documentation. I am working with documentation team to add documentation for Spanner Traces as well along with metrics. Once we have that we can further expand it to add documentation for JDBC as well

```

### Example Using an OpenTelemetry instance in Properties
Create and register a global OpenTelemetry object before creating a JDBC connection.

Choose a reason for hiding this comment

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

Here as well. global OT is not used here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

}
```

## Traces

Choose a reason for hiding this comment

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

This only talks about JDBC traces, but in reality customer would see way more traces coming from spanner library.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This text 'pretends' that all the traces come from the JDBC driver (although some of the text also mentions the client lib). I think that for a customer it is easier to read/understand if we mainly talk about JDBC here, as most users who use the JDBC driver are not using the client library. The fact that they are layered is not necessarily a known fact for everyone.
The traces that are described here also include the traces from the client library (e.g. everything in point 2 and 3 are actually collected by the client library)

Choose a reason for hiding this comment

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

What I meant is customer would also see CloudSpanner.ReadWriteTransaction as a child span to JDBC.ReadWriteTransaction which is not mentioned here.

But I understand adding too much details here can also be confusing for customers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, those are the ones that are described in point 2 below, but the text did not call out what they are named. I added that to the text to make it easier for a reader to make the link.

Create an OpenTelemetry object and supply it as part of the properties for a JDBC connection.

```java
TraceConfiguration traceConfiguration = TraceConfiguration.builder().setProjectId("my-project").build();

Choose a reason for hiding this comment

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

We can remove this code from here as we have done in readme-partials file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually kept the full code here intentionally, as having a full copy-pastable example for each of the use cases makes it easier for a reader to quickly get started if they want to use one or the other.

}
```

## Traces

Choose a reason for hiding this comment

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

What I meant is customer would also see CloudSpanner.ReadWriteTransaction as a child span to JDBC.ReadWriteTransaction which is not mentioned here.

But I understand adding too much details here can also be confusing for customers.

@@ -0,0 +1,257 @@
# Spanner JDBC Driver - Latency Debugging Guide

The Spanner JDBC driver supports OpenTelemetry tracing. Tracing can be used to investigate slow

Choose a reason for hiding this comment

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

Yes, we currently don't have a very good observability documentation. I am working with documentation team to add documentation for Spanner Traces as well along with metrics. Once we have that we can further expand it to add documentation for JDBC as well

@olavloite olavloite added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jun 27, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 27, 2024
@olavloite olavloite merged commit 1485a04 into main Jun 27, 2024
24 of 26 checks passed
@olavloite olavloite deleted the open-telemetry-tracing branch June 27, 2024 10:17
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 27, 2024
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 27, 2024
🤖 I have created a release *beep* *boop*
---


## [2.20.0](https://togithub.com/googleapis/java-spanner-jdbc/compare/v2.19.4...v2.20.0) (2024-06-27)


### Features

* Add OpenTelemetry tracing ([#1568](https://togithub.com/googleapis/java-spanner-jdbc/issues/1568)) ([1485a04](https://togithub.com/googleapis/java-spanner-jdbc/commit/1485a04272c270851468254bebffe4f7d846f17c))


### Dependencies

* Update dependency com.google.api.grpc:proto-google-cloud-trace-v1 to v2.45.0 ([#1659](https://togithub.com/googleapis/java-spanner-jdbc/issues/1659)) ([0491712](https://togithub.com/googleapis/java-spanner-jdbc/commit/0491712babfdc7088358f28c4f261e3ec6dd0d2b))
* Update dependency com.google.cloud:google-cloud-spanner-bom to v6.70.0 ([#1657](https://togithub.com/googleapis/java-spanner-jdbc/issues/1657)) ([83aaec1](https://togithub.com/googleapis/java-spanner-jdbc/commit/83aaec17b0c28ac9ef45e99eb878d40572124f3f))
* Update dependency com.google.cloud:google-cloud-trace to v2.45.0 ([#1661](https://togithub.com/googleapis/java-spanner-jdbc/issues/1661)) ([648224c](https://togithub.com/googleapis/java-spanner-jdbc/commit/648224cfb2310d80f781386be64be00157133a30))
* Update dependency com.google.cloud:sdk-platform-java-config to v3.32.0 ([#1654](https://togithub.com/googleapis/java-spanner-jdbc/issues/1654)) ([503e281](https://togithub.com/googleapis/java-spanner-jdbc/commit/503e281106910dc9352deeaf790097e495df127b))
* Update dependency org.springframework.boot:spring-boot-starter-data-jdbc to v3.3.1 ([#1648](https://togithub.com/googleapis/java-spanner-jdbc/issues/1648)) ([4df665f](https://togithub.com/googleapis/java-spanner-jdbc/commit/4df665fa3a0b65757fc1c00fcf7a0fbd6c0c5d2e))
* Update dependency org.springframework.boot:spring-boot-starter-parent to v3.3.1 ([#1649](https://togithub.com/googleapis/java-spanner-jdbc/issues/1649)) ([efe87a6](https://togithub.com/googleapis/java-spanner-jdbc/commit/efe87a6016b8d61c42f4a4ad5b7ead180545d24e))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner-jdbc API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants