Skip to content

Conversation

@aryan735
Copy link

What

  • Enforces validation that OTLP gRPC endpoints must not include a URL path
  • Surfaces invalid OTEL_EXPORTER_OTLP_ENDPOINT errors during exporter creation

Why

  • gRPC OTLP exporters require host:port only
  • Endpoints like /v1/traces are HTTP-specific and invalid for gRPC
  • Prevents silent misconfiguration

How

  • Ensure loadEnvConfig() is invoked during exporter initialization
  • Add tests covering valid and invalid endpoint formats

Tests

  • TestEndpointWithPath_ShouldFail
  • TestEndpointWithoutPath_ShouldPass
  • Test available in pkg : otlptracegrpc
  • Test file name : grpc_test.go

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 25, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@dmathieu
Copy link
Member

While this validation makes sense, the approach taken is really hard to read and doesn't follow the common Go patterns.
We shouldn't need a global variable for this. And otlptrace.New() already does some validation. This one can happen here.

@aryan735
Copy link
Author

While this validation makes sense, the approach taken is really hard to read and doesn't follow the common Go patterns. We shouldn't need a global variable for this. And otlptrace.New() already does some validation. This one can happen here.

Thanks for the review.
I’ve refactored the validation to remove the global error state and moved the OTLP gRPC endpoint path check into New() alongside existing validation.
The fix now returns an error for gRPC endpoints containing a path and passes the updated tests.

Please let me know if any further changes are needed.

Copy link
Member

@XSAM XSAM left a comment

Choose a reason for hiding this comment

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

unix://foo/bar (where the bar is the path here) is a valid gRPC endpoint. While I am not sure whether the current implementation supports unix, I am not sure this PR puts unix support in the right direction.

Related:

return otlptrace.NewUnstarted(NewClient(opts...))
}

func validateGRPCEndpontFromEnv() error {
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure it is necessary to validate a grpc endpoint by ourselves. Should we just pass it to the gRPC client and let it decide?

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.

3 participants