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

Support OTEL_EXPORTER_OTLP_LOGS_INSECURE and OTEL_EXPORTER_OTLP_INSECURE environments in grpc exporter #5739

Merged
merged 16 commits into from
Sep 6, 2024

Conversation

amanakin
Copy link
Contributor

@amanakin amanakin commented Aug 24, 2024

Closes #5719

In this commit I add OTEL_EXPORTER_OTLP_LOGS_INSECURE and OTEL_EXPORTER_OTLP_INSECURE env options to otlploggrpc.Exporter.

Now insecure option is fetched from env endpoint value (OTEL_EXPORTER_OTLP_LOGS_ENDPOINT/OTEL_EXPORTER_OTLP_ENDPOINT).
According to spec:

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.

So with current behavior we have several problems:

  • If default endpoint is used, we can't use insecure connection (with setting OTEL_EXPORTER_OTLP_INSECURE).
  • If endpoint provided with option without scheme (e.g. WithEndpoint) we can't use insecure connection with env settings.
  • If endpoint provided with env variable without scheme (e.g. //env.endpoint:8080/) we can't use insecure connection.

This commit fixes this.

The same problem with otlploghttp.Exporter, and probably it should be fixed there too.
I'm open to suggestions on how to fix the current behavior in a more elegant way.

Copy link

codecov bot commented Aug 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.6%. Comparing base (fb7cc02) to head (1703736).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #5739   +/-   ##
=====================================
  Coverage   84.5%   84.6%           
=====================================
  Files        272     272           
  Lines      22759   22778   +19     
=====================================
+ Hits       19253   19271   +18     
  Misses      3166    3166           
- Partials     340     341    +1     

see 2 files with indirect coverage changes

@amanakin
Copy link
Contributor Author

amanakin commented Sep 2, 2024

Can anyone take a look? 👉👈

exporters/otlp/otlplog/otlploggrpc/config.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@amanakin amanakin changed the title Add OTEL_EXPORTER_OTLP_LOGS_INSECURE and OTEL_EXPORTER_OTLP_INSECURE exporter env options Support OTEL_EXPORTER_OTLP_LOGS_INSECURE and OTEL_EXPORTER_OTLP_INSECURE environments in grpc exporter Sep 5, 2024
@amanakin amanakin requested a review from XSAM September 5, 2024 07:32
@dmathieu dmathieu merged commit 8dca9cc into open-telemetry:main Sep 6, 2024
32 checks passed
@XSAM XSAM added this to the v1.30.0 milestone Sep 9, 2024
XSAM added a commit that referenced this pull request Sep 10, 2024
### Added

- Support `OTEL_EXPORTER_OTLP_LOGS_INSECURE` and
`OTEL_EXPORTER_OTLP_INSECURE` environments in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc`. (#5739)
- The `WithResource` option for `NewMeterProvider` now merges the
provided resources with the ones from environment variables. (#5773)
- The `WithResource` option for `NewLoggerProvider` now merges the
provided resources with the ones from environment variables. (#5773)
- Add UTF-8 support to `go.opentelemetry.io/otel/exporters/prometheus`.
(#5755)
### Fixed

- Fix memory leak in the global `MeterProvider` when identical
instruments are repeatedly created. (#5754)
- Fix panic instruments creation when setting meter provider. (#5758)
- Fix panic on instruments creation when setting meter provider. (#5758)
- Fix an issue where `SetMeterProvider` in `go.opentelemetry.io/otel`
might miss the delegation for instruments and registries. (#5780)

### Removed

- Drop support for [Go 1.21](https://go.dev/doc/go1.21). (#5736, #5740,
#5800)
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.

OTLP Log gRPC ignores OTEL_EXPORTER_OTLP_INSECURE=true
3 participants