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

Use https scheme ins grpc env var exporter examples #2863

Merged
merged 3 commits into from
Jun 12, 2023
Merged

Conversation

cartermp
Copy link
Contributor

fixes #2861

@cartermp cartermp requested a review from a team as a code owner June 11, 2023 03:43
@hu6360567
Copy link

If the endpoint always comes with the scheme, then what's the use of OTEL_EXPORTER_OTLP_INSECURE for?

@svrnm
Copy link
Member

svrnm commented Jun 12, 2023

If the endpoint always comes with the scheme, then what's the use of OTEL_EXPORTER_OTLP_INSECURE for?

insecure allows self-signed keys

@hu6360567
Copy link

hu6360567 commented Jun 12, 2023

If the endpoint always comes with the scheme, then what's the use of OTEL_EXPORTER_OTLP_INSECURE for?

insecure allows self-signed keys

Is it described in any specification?
Seems OTEL_EXPORTER_OTLP_INSECURE in python would affect scheme, rather than validating server certificate.

The OTEL_EXPORTER_OTLP_INSECURE represents whether to enable client transport security for gRPC requests. A scheme of https takes precedence over this configuration setting. Default: False

Also, for signed keys, there should be a way to specify the key server used to verify the server identity, rather than just ignoring the certificate from server.

Will this PR cause a compatibility problem?

I'm fine with the format, as long as the value can be accepted accross the ecosystem.

@svrnm
Copy link
Member

svrnm commented Jun 12, 2023

If the endpoint always comes with the scheme, then what's the use of OTEL_EXPORTER_OTLP_INSECURE for?

insecure allows self-signed keys

Is it described in any specification? Seems OTEL_EXPORTER_OTLP_INSECURE in python would affect scheme, rather than validating server certificate.

The OTEL_EXPORTER_OTLP_INSECURE represents whether to enable client transport security for gRPC requests. A scheme of https takes precedence over this configuration setting. Default: False

Also, for signed keys, there should be a way to specify the key server used to verify the server identity, rather than just ignoring the certificate from server.

Will this PR cause a compatibility problem?

I'm fine with the format, as long as the value can be accepted accross the ecosystem.

May bad, I was wrong here!

From the spec [1]:

  • Insecure: Whether to enable client transport security for the exporter's gRPC connection. This option only applies to OTLP/gRPC when an endpoint is provided without the http or https scheme - OTLP/HTTP always uses the scheme provided for the endpoint. Implementations MAY choose to not implement the insecure option if it is not required or supported by the underlying gRPC client implementation.

I should have done more reading on the issue, I have to admit I was not even aware of that combination of environment variables. From a documentation standpoint having the schema is our best option, it leads to the least issues, since languages MUST support that schema (similar to what @jack-berg stated here about Java: open-telemetry/opentelemetry-java#5517 (comment)).

Outside documentation, if any language does not support schema, this is a bug and needs to be fixed. We can call that out in the documentation until the bug is fixed.

@hu6360567
Copy link

It seems the python implementation can accept the export variable with shceme, but when it has conflict with our application when auto-instrument enabled, I'll inspect it later.

@cartermp
Copy link
Contributor Author

@hu6360567 do you have any issue open here? https://github.com/open-telemetry/opentelemetry-python-contrib

I would bet they're keen on fixing the issue.

@cartermp cartermp merged commit 5cd333a into main Jun 12, 2023
8 checks passed
@cartermp cartermp deleted the cartermp-patch-1 branch June 12, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to clarify OTEL_EXPORTER_OTLP_TRACES_ENDPOINT format forOTLP Exporter Configuration
3 participants