open-telemetry / opentelemetry-java

OpenTelemetry Java SDK
https://opentelemetry.io
Apache License 2.0
1.97k stars 814 forks source link

enable JKS certificate format in opentelemetry sdk autoconfigure #6314

Open Maikels opened 6 months ago

Maikels commented 6 months ago

Hi,

When using the java library we can set our jks certificates with following method:


KeyStore ks = KeyStore.getInstance("JKS");
ks.load(getResourceAsInputStream(tlsKeyStore), tlsKeyStorePassword.toCharArray());

KeyStore ts = KeyStore.getInstance("JKS");
ts.load(getResourceAsInputStream(tlsTruststore), tlsTrustStorePassword.toCharArray());

builder.setEndpoint(String.format(ENDPOINT_FORMAT, agentHost, agentPort))
            .setTimeout(timeout, TimeUnit.SECONDS)
            .setClientTls(ks.getKey("private-key", tlsKeyPassword.toCharArray()).getEncoded(), ks.getCertificate("cert-chain").getEncoded())
            .setTrustedCertificates(ts.getCertificate("collector-cert").getEncoded())
            .build();

Can we make it possible to do this with the autoconfigure as well? Right now I'm using PEM format certificates, which works (OTEL_EXPORTER_OTLP_CERTIFICATE / OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE) but in the future we need to use JKS files.

Thanks

jack-berg commented 5 months ago

The PEM format for OTEL_EXPORTER_OTLP_CERTIFICATE and OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE comes from the spec here: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md

Normally I'd say that we're blocked on the spec, and you should world to update the spec to include jks. However, given that JKS is a java specific format, its unlikely to be added. @open-telemetry/java-approvers WDYT of this? The spec is pretty specific on the PEM format. There may be complexities with supporting any other format, let alone a java specific one like JDK, since we minimally need to be able to determine what type of key we're dealing with.

jkwatson commented 5 months ago

Can we add a java-specific env var? Is there precedence for that?

jack-berg commented 5 months ago

Yes there is precedent for that: https://github.com/open-telemetry/opentelemetry-java/tree/main/sdk-extensions/autoconfigure#disabling-automatic-resourceproviders