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

Review OpenTelemetry and OTLP support for consistency #41460

Closed
wilkinsona opened this issue Jul 12, 2024 · 8 comments
Closed

Review OpenTelemetry and OTLP support for consistency #41460

wilkinsona opened this issue Jul 12, 2024 · 8 comments
Assignees
Labels
type: task A general task
Milestone

Comments

@wilkinsona
Copy link
Member

wilkinsona commented Jul 12, 2024

Packages

  • org.springframework.boot.actuate.autoconfigure.logging.opentelemetry
  • org.springframework.boot.actuate.autoconfigure.logging.opentelemetry.otlp
  • org.springframework.boot.actuate.autoconfigure.metrics.export.otlp
  • org.springframework.boot.actuate.autoconfigure.opentelemetry
  • org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp
  • org.springframework.boot.actuate.autoconfigure.tracing.otlp

Properties

  • management.otlp.logging.compression
  • management.otlp.logging.endpoint
  • management.otlp.logging.headers.*
  • management.otlp.logging.timeout
  • management.otlp.metrics.export.aggregation-temporality
  • management.otlp.metrics.export.base-time-unit
  • management.otlp.metrics.export.batch-size
  • management.otlp.metrics.export.connect-timeout
  • management.otlp.metrics.export.enabled
  • management.otlp.metrics.export.headers.*
  • management.otlp.metrics.export.read-timeout
  • management.otlp.metrics.export.step
  • management.otlp.metrics.export.url
  • management.otlp.tracing.compression
  • management.otlp.tracing.endpoint
  • management.otlp.tracing.headers.*
  • management.otlp.tracing.timeout
  • management.otlp.tracing.transport

Connection Details

  • OtlpLoggingConnectionDetails
    • getUrl(Transport) (management.otlp.logging.endpoint)
  • OtlpMetricsConnectionDetails
    • getUrl() (management.otlp.metrics.export.url)
  • OtlpTracingConnectionDetails
    • getUrl(Transport) (management.otlp.tracing.endpoint)
@wilkinsona wilkinsona added this to the 3.4.x milestone Jul 12, 2024
@wilkinsona wilkinsona added type: task A general task status: pending-design-work Needs design work before any code can be developed labels Jul 12, 2024
mhalbritter added a commit that referenced this issue Aug 8, 2024
@mhalbritter
Copy link
Contributor

mhalbritter commented Sep 6, 2024

I got something started here: https://github.com/mhalbritter/spring-boot/tree/mh/41460-review-opentelemetry-and-otlp-support-for-consistency

It moves the stuff under org.springframework.boot.actuate.autoconfigure.logging.opentelemetry to org.springframework.boot.actuate.autoconfigure.logging and the stuff under org.springframework.boot.actuate.autoconfigure.logging.opentelemetry.otlp to org.springframework.boot.actuate.autoconfigure.logging.otlp.

The package structure is now the same as for tracing.

The properties for tracing and logging look consistent too (management.otlp.tracing.transport has no logging equivalent (yet)).

@mhalbritter
Copy link
Contributor

mhalbritter commented Sep 6, 2024

management.otlp.metrics.export.url is inconsistent. Most of the other export URLs are named uri, e.g. management.datadog.metrics.export.uri.

The underlying method on OtlpConfig is called url().

We could either fix that to be consistent to the other metrics properties (management.otlp.metrics.export.uri) or to be consistent to the other OTLP properties (management.otlp.metrics.export.endpoint).

@mhalbritter
Copy link
Contributor

We could add read-timeout and connect-timeout to our tracing and logging properties to make them consistent to the metrics properties.

Right now, management.otlp.logging.timeout and management.otlp.tracing.timeout call setTimeout on the Otel builder, and the JavaDoc says:

Sets the maximum time to wait for the collector to process an exported batch of spans.

which lead me to think that this is the read timeout. There's also a setConnectTimeout on the builders.

@mhalbritter
Copy link
Contributor

Regarding org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp: not sure if otlp is good under opentelemetry or if we should move it to org.springframework.boot.actuate.autoconfigure.otlp.

@mhalbritter
Copy link
Contributor

I don't see a way to add transport or compression to management.otlp.metrics.export. They configure the underlying OtlpMeterRegistry and this only supports gRPC over http and doesn't compress anything.

@mhalbritter mhalbritter added the for: team-meeting An issue we'd like to discuss as a team to make progress label Sep 6, 2024
@mhalbritter mhalbritter removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Oct 8, 2024
@mhalbritter mhalbritter self-assigned this Oct 8, 2024
@mhalbritter
Copy link
Contributor

mhalbritter commented Oct 8, 2024

Regarding the read and connect timeout: While we can add connect timeout properties to OtlpLoggingProperties and OtlpTracingProperties (which call .setConnectTimeout on the OtlpExporterBuilder), supporting read timeout is not possible.

The method we call at the moment, .setTimeout, delegates at the end to OkHttpClient.Builder.callTimeout, which is the time for "resolving DNS, connecting, writing the request body, server processing, as well as reading the response body.".

There's no method to set the read timeout.

@mhalbritter
Copy link
Contributor

mhalbritter commented Oct 8, 2024

After a refactoring, it now looks like this:

org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp is gone. The Transport and Compression enums have been copied to metrics and logging directly.

Logging

  • OpenTelemetry specific classes in org.springframework.boot.actuate.autoconfigure.logging
  • OTLP specific classes in org.springframework.boot.actuate.autoconfigure.logging.otlp

Properties

From org.springframework.boot.actuate.autoconfigure.logging.otlp.OtlpLoggingProperties

  • management.otlp.logging.compression
  • management.otlp.logging.connect-timeout
  • management.otlp.logging.endpoint
  • management.otlp.logging.headers.*
  • management.otlp.logging.timeout
  • management.otlp.logging.transport

Metrics

OTLP classes in org.springframework.boot.actuate.autoconfigure.metrics.export.otlp

Properties

From org.springframework.boot.actuate.autoconfigure.metrics.export.otlp.OtlpMetricsProperties

  • management.otlp.metrics.export.aggregation-temporality
  • management.otlp.metrics.export.base-time-unit
  • management.otlp.metrics.export.batch-size
  • management.otlp.metrics.export.connect-timeout
  • management.otlp.metrics.export.enabled
  • management.otlp.metrics.export.headers.*
  • management.otlp.metrics.export.read-timeout
  • management.otlp.metrics.export.step
  • management.otlp.metrics.export.url

Tracing

  • OpenTelemetry specific classes in org.springframework.boot.actuate.autoconfigure.tracing
  • OTLP specific classes in org.springframework.boot.actuate.autoconfigure.tracing.otlp

Properties

From org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingProperties

  • management.otlp.tracing.compression
  • management.otlp.tracing.connect-timeout
  • management.otlp.tracing.endpoint
  • management.otlp.tracing.export.enabled
  • management.otlp.tracing.headers.*
  • management.otlp.tracing.timeout
  • management.otlp.tracing.transport

I've also renamed org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration to OtlpTracingAutoConfiguration.

mhalbritter added a commit that referenced this issue Oct 8, 2024
- Rename OtlpProperties to OtlpTracingProperties

See gh-41460
See gh-42528
@mhalbritter mhalbritter removed the status: pending-design-work Needs design work before any code can be developed label Oct 8, 2024
@mhalbritter mhalbritter modified the milestones: 3.4.x, 3.4.0-RC1 Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

2 participants