feat(spanner): Update OpenTelemetry OTLP trace sample to use Google C…#10251
feat(spanner): Update OpenTelemetry OTLP trace sample to use Google C…#10251surbhigarg92 wants to merge 2 commits intomainfrom
Conversation
product-auto-label
Bot
added
api: spanner
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the OpenTelemetry Spanner sample to include authentication headers for the OTLP exporter and modifies the default configuration to use the OTLP endpoint with a full sampling rate. Feedback suggests reverting specific testing values, such as project IDs and service names, to generic placeholders to ensure the code sample remains professional and reusable for a public audience.
surbhigarg92
force-pushed
the
otlp_sample_spanner
branch
from
bdcf525 to
aa30392
Compare
…loud OTLP endpoint
surbhigarg92
force-pushed
the
otlp_sample_spanner
branch
from
aa30392 to
fe5e51c
Compare
| static boolean useCloudTraceExporter = true; // Replace to false for OTLP | ||
| static String otlpEndpoint = "http://localhost:4317"; // Replace with your OTLP endpoint | ||
| static boolean useCloudTraceExporter = false; // Replace to true for Cloud Trace exporter | ||
| static String otlpEndpoint = |
There was a problem hiding this comment.
The customer's OTLP configuration should be respected (see relevant OTel environment variables configuring the exporter). However, auth and project ID should get automatically injected/added if the endpoint is the Telemetry API or one of its regional variants.
There was a problem hiding this comment.
Sure changed it to fetch from OTEL_EXPORTER_OTLP_ENDPOINT/OTEL_EXPORTER_OTLP_TRACES_ENDPOINT
Have used google telemetry endpoint as default.
| Resource.getDefault() | ||
| .merge( | ||
| Resource.builder() | ||
| .put("service.name", "My App") |
There was a problem hiding this comment.
Why "My App"? The service name really ought to be supplied by the program, either through an explicit parameter or through environment variables.
Note that there are standard OTel environment variables that are typically used to supply this.
| GoogleCredentials credentials = | ||
| GoogleCredentials.getApplicationDefault() | ||
| .createScoped(Collections.singleton("https://www.googleapis.com/auth/trace.append")); | ||
| OtlpGrpcSpanExporter otlpGrpcSpanExporter = | ||
| OtlpGrpcSpanExporter | ||
| .builder() | ||
| .setEndpoint(otlpEndpoint) // Replace with your OTLP endpoint | ||
| OtlpGrpcSpanExporter.builder() | ||
| .setEndpoint(otlpEndpoint) |
There was a problem hiding this comment.
The credentials and header logic should be conditioned on the endpoint matching the GCP Telemetry API. But that endpoint should not be hardcoded. It should be easy for customers to use the library with a collector of their choice.
There was a problem hiding this comment.
Added this as a conditional logic for "telemetry.googleapis.com"
| SpannerOptions options = SpannerOptions.newBuilder() | ||
| .setOpenTelemetry(openTelemetry) | ||
| .build(); | ||
| SpannerOptions options = SpannerOptions.newBuilder().setOpenTelemetry(openTelemetry).build(); |
There was a problem hiding this comment.
Can we remove this change? This looks more cleaner
| if (otlpEndpoint == null) { | ||
| otlpEndpoint = System.getenv("OTEL_EXPORTER_OTLP_ENDPOINT"); | ||
| } | ||
| if (otlpEndpoint == null) { |
There was a problem hiding this comment.
I think if "OTEL_EXPORTER" or "OTEL_TRACES_EXPORTER" are set to "gcp" or if environment variable "GOOGLE_CLOUD_TRACING_ENABLED" is set to "true", then you can fallback like this.
Otherwise, I think you should fallback to the default behavior of Open Telemetry. If "OTEL_EXPORTER" or "OTEL_TRACES_EXPORTER" is set to "otlp" and neither "OTEL_EXPORTER_OTLP_ENDPOINT" nor "OTEL_OTLP_EXPORTER_TRACES_ENDPOINT" are set, then the default is to use "http://localhost:4317" for gRPC and "http://localhost:4318" for HTTP. See:
https://opentelemetry.io/docs/languages/sdk-configuration/otlp-exporter/
|
|
||
| // Add authentication only if the endpoint is the Google Cloud Telemetry API. | ||
| // The standard endpoint is telemetry.googleapis.com | ||
| if (otlpEndpoint.contains("telemetry.googleapis.com")) { |
There was a problem hiding this comment.
Note that you also need to match regionalized addresses:
telemetry.googleapis.com (global)
telemetry.us-central1.rep.googleapis.com (regionalized in cloud region us-central1)
telemetry.us-east1.rep.googleapis.com (regionalized in us-east1)
...
Prefixed with "https://" or suffixed with ":443" (optionally with "dns:// " as a prefix). Ex:
telemetry.googleapis.com
https://telemetry.googleapis.com
telemetry.googleapis.com:443
dns://telemetry.googleapis.com:443
https://telemetry.us-central1.rep.googleapis.com
telemetry.us-central1.rep.googleapis.com:443
...
However, you also want this match to apply only to the domain (it would be a mistake to match "otherdomain.com/proxy/telemetry.googleapis.com").
| .addSpanProcessor(otlpGrpcSpanProcessor) | ||
| .addSpanProcessor(BatchSpanProcessor.builder(exporterBuilder.build()).build()) | ||
| .setResource(resource) | ||
| .setSampler(Sampler.traceIdRatioBased(0.1)) |
There was a problem hiding this comment.
Use the OTEL_TRACES_SAMPLER flag to control sampling behavior?
https://opentelemetry.io/docs/languages/sdk-configuration/general/#otel_traces_sampler
3 participants
…loud OTLP endpoint
Description