Skip to content

feat(otlp-http): improve endpoint handling, add User-Agent header, an…#2074

Open
arjun-rajappa wants to merge 1 commit intoopen-telemetry:mainfrom
arjun-rajappa:add-tests-otlp-http
Open

feat(otlp-http): improve endpoint handling, add User-Agent header, an…#2074
arjun-rajappa wants to merge 1 commit intoopen-telemetry:mainfrom
arjun-rajappa:add-tests-otlp-http

Conversation

@arjun-rajappa
Copy link
Copy Markdown
Contributor

Fixes #1667

  • Add spec-compliant default User-Agent header with exporter version and Ruby runtime info
  • Fix endpoint path construction to properly handle OTEL_EXPORTER_OTLP_ENDPOINT with/without trailing slashes
  • Implement proper endpoint precedence: explicit endpoint > OTEL_EXPORTER_OTLP_TRACES_ENDPOINT > OTEL_EXPORTER_OTLP_ENDPOINT
  • Add metrics for compressed and uncompressed message sizes
  • Add specific error handling and logging for 404 responses
  • Refactor endpoint and header preparation into dedicated methods
  • Prevent header mutation by duplicating Hash parameters
  • Add comprehensive test coverage for endpoint path handling and User-Agent header

…d enhance metrics

- Add spec-compliant default User-Agent header with exporter version and Ruby runtime info
- Fix endpoint path construction to properly handle OTEL_EXPORTER_OTLP_ENDPOINT with/without trailing slashes
- Implement proper endpoint precedence: explicit endpoint > OTEL_EXPORTER_OTLP_TRACES_ENDPOINT > OTEL_EXPORTER_OTLP_ENDPOINT
- Add metrics for compressed and uncompressed message sizes
- Add specific error handling and logging for 404 responses
- Refactor endpoint and header preparation into dedicated methods
- Prevent header mutation by duplicating Hash parameters
- Add comprehensive test coverage for endpoint path handling and User-Agent header

Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
private_constant(:ERROR_MESSAGE_INVALID_HEADERS)

def initialize(endpoint: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_ENDPOINT', 'OTEL_EXPORTER_OTLP_ENDPOINT', default: 'http://localhost:4318/v1/traces'),
DEFAULT_USER_AGENT = "OTel-OTLP-Exporter-Ruby/#{OpenTelemetry::Exporter::OTLP::HTTP::VERSION} Ruby/#{RUBY_VERSION} (#{RUBY_PLATFORM}; #{RUBY_ENGINE}/#{RUBY_ENGINE_VERSION})".freeze
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

private_constant?

compression: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_COMPRESSION', 'OTEL_EXPORTER_OTLP_COMPRESSION', default: 'gzip'),
timeout: OpenTelemetry::Common::Utilities.config_opt('OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', 'OTEL_EXPORTER_OTLP_TIMEOUT', default: 10),
metrics_reporter: nil)
raise ArgumentError, "invalid url for OTLP::Exporter #{endpoint}" unless OpenTelemetry::Common::Utilities.valid_url?(endpoint)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why remove this validation?

Comment on lines -141 to +142
request.body = if @compression == 'gzip'
request.add_field('Content-Encoding', 'gzip')
Zlib.gzip(bytes)
else
bytes
end
if @compression == 'gzip'
request.add_field('Content-Encoding', 'gzip')
body = Zlib.gzip(bytes)
@metrics_reporter.record_value('otel.otlp_exporter.message.compressed_size', value: body.bytesize)
else
body = bytes
end
request.body = body
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is no metrics support at this point, so I don't think it needs to change here.
value = if ... else ... is more idiomatic in Ruby.

redo if backoff?(retry_count: retry_count += 1, reason: response.code)
FAILURE
when Net::HTTPNotFound
log_request_failure(response.code)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is no metrics support, OpenTelemetry.handle_error seems enough.

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.

Make otlp-http, otlp-grpc, and otlp-common gems production-ready

2 participants