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.

Sign up for GitHub

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

Jump to bottom

otlp endpoint required on loadbalancingexporter config #31371

Closed
kurayama opened this issue Feb 21, 2024 · 3 comments · Fixed by #31381
Closed

otlp endpoint required on loadbalancingexporter config #31371

kurayama opened this issue Feb 21, 2024 · 3 comments · Fixed by #31381
Assignees
@jpkrohling
Labels
bug Something isn't working exporter/loadbalancing

Comments

@kurayama
Copy link

Component(s)

exporter/loadbalancing

What happened?

Description

Since open-telemetry/opentelemetry-collector#9523, the endpoint inside otlp became mandatory and for it to work we needed to add a bogus endpoint that is replaced by the resolver later on.
The documentation states:

The otlp property configures the template used for building the OTLP exporter. Refer to the OTLP Exporter documentation for information on which options are available. Note that the endpoint property should not be set and will be overridden by this exporter with the backend endpoint.

Which is not true anymore.

Steps to Reproduce

Expected Result

Actual Result

Collector version

v0.95.0

Environment information

Environment

0.95 tag for the dockerhub image

OpenTelemetry Collector configuration

exporters:
  loadbalancing:
    protocol:
      otlp:
        timeout: 1s
        tls:
          insecure: true
    resolver:
      k8s:
        service: <our-internal-endpoint>

Log output

Error: invalid configuration: exporters::loadbalancing: requires a non-empty "endpoint"
2024/02/21 18:08:46 collector server run finished with error: invalid configuration: exporters::loadbalancing: requires a non-empty "endpoint"

Additional context

No response
@kurayama kurayama added bug Something isn't working needs triage New item requiring triage labels Feb 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mrsimo
Copy link

mrsimo commented Feb 22, 2024

We're seeing this as well. This release has broken all configurations using the loadbalancingexporter. Would it be safe to add a bogus endpoint attribute to the otlp block?

@jpkrohling
Copy link
Member

For now, yes, but I'll discuss this with the other collector maintainers to see if it's feasible to revert that change and produce a patch release. For now, please keep using 0.94.0 unless you have strong reasons to use 0.95.0.

@jpkrohling jpkrohling self-assigned this Feb 22, 2024
@jpkrohling jpkrohling removed the needs triage New item requiring triage label Feb 22, 2024
@jpkrohling jpkrohling mentioned this issue Feb 22, 2024
Merged
jpkrohling added a commit to jpkrohling/opentelemetry-collector-contrib that referenced this issue Feb 22, 2024
@jpkrohling
[exporter/loadbalancing] Set a placeholder endpoint
beead1d 
As part of open-telemetry/opentelemetry-collector#9523, the OTLP Exporter configuration used by the load balancing exporter is being validates automatically. However, the endpoint is the only thing users should NOT set, as it will be set at a later point in time by the load balancer.

This PR sets a placeholder value for the endpoint so that the validation passes.

Fixes open-telemetry#31371

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
jpkrohling added a commit that referenced this issue Feb 22, 2024
@jpkrohling
[exporter/loadbalancing] Set a placeholder endpoint (#31381)
0c378db 
As part of
open-telemetry/opentelemetry-collector#9523, the
OTLP Exporter configuration used by the load balancing exporter is being
validates automatically. However, the endpoint is the only thing users
should NOT set, as it will be set at a later point in time by the load
balancer.

This PR sets a placeholder value for the endpoint so that the validation
passes.

Fixes #31371

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

---------

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@willthames willthames mentioned this issue Feb 26, 2024
Closed
jpkrohling added a commit to jpkrohling/opentelemetry-collector-contrib that referenced this issue Feb 26, 2024
@jpkrohling
[exporter/loadbalancing] Set a placeholder endpoint (open-telemetry#3…
a37503b 
…1381)

As part of
open-telemetry/opentelemetry-collector#9523, the
OTLP Exporter configuration used by the load balancing exporter is being
validates automatically. However, the endpoint is the only thing users
should NOT set, as it will be set at a later point in time by the load
balancer.

This PR sets a placeholder value for the endpoint so that the validation
passes.

Fixes open-telemetry#31371

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

---------

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
This was referenced Feb 27, 2024
Closed
Open
@atmask atmask mentioned this issue Feb 27, 2024
Merged
@andrzej-stencel andrzej-stencel mentioned this issue Feb 27, 2024
Closed
TylerHelmuth pushed a commit that referenced this issue Feb 28, 2024
@atmask
[loadbalancingexporter] Set the default otlp port 4317 in factory and…
cea1de2 
… unit test (#31425)

**Description:** 
As described in the otel-collector [issue
9505](open-telemetry/opentelemetry-collector#9505),
the otlpexporter does not function correctly if no port is defined. To
resolve this, the otlp config validation has been updated to fail fast
when the endpoint within an otlp config does not have a port specified.

The loadbalancingexporter config has the otlp exporter config as a
dependency, however, the loadbalancing exporter does not define a port
on the otlpexporter config in two places:
- default config from factory
- testdata contents

This is currently a blocker to the contrib tests for the
[PR](open-telemetry/opentelemetry-collector#9632)
to resolve issue 9505

Relates to:
open-telemetry/opentelemetry-collector#9523

#31371

#31381


**Link to tracking Issue:** 
otel-collector-contrib: [issue
31426](#31426)
Arises from otel-collector [issue
9505](open-telemetry/opentelemetry-collector#9505)

**Testing:** Used `replace` to test loadbalancingexporter changes pass
tests successfully when using the otlpexporter changes from
[PR](open-telemetry/opentelemetry-collector#9632)
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this issue Mar 13, 2024
@jpkrohling @XinRanZhAWS
[exporter/loadbalancing] Set a placeholder endpoint (open-telemetry#3…
5f88275 
…1381)

As part of
open-telemetry/opentelemetry-collector#9523, the
OTLP Exporter configuration used by the load balancing exporter is being
validates automatically. However, the endpoint is the only thing users
should NOT set, as it will be set at a later point in time by the load
balancer.

This PR sets a placeholder value for the endpoint so that the validation
passes.

Fixes open-telemetry#31371

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

---------

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this issue Mar 13, 2024
@atmask @XinRanZhAWS
[loadbalancingexporter] Set the default otlp port 4317 in factory and…
8aa6bec 
… unit test (open-telemetry#31425)

**Description:** 
As described in the otel-collector [issue
9505](open-telemetry/opentelemetry-collector#9505),
the otlpexporter does not function correctly if no port is defined. To
resolve this, the otlp config validation has been updated to fail fast
when the endpoint within an otlp config does not have a port specified.

The loadbalancingexporter config has the otlp exporter config as a
dependency, however, the loadbalancing exporter does not define a port
on the otlpexporter config in two places:
- default config from factory
- testdata contents

This is currently a blocker to the contrib tests for the
[PR](open-telemetry/opentelemetry-collector#9632)
to resolve issue 9505

Relates to:
open-telemetry/opentelemetry-collector#9523

open-telemetry#31371

open-telemetry#31381


**Link to tracking Issue:** 
otel-collector-contrib: [issue
31426](open-telemetry#31426)
Arises from otel-collector [issue
9505](open-telemetry/opentelemetry-collector#9505)

**Testing:** Used `replace` to test loadbalancingexporter changes pass
tests successfully when using the otlpexporter changes from
[PR](open-telemetry/opentelemetry-collector#9632)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jpkrohling jpkrohling

Labels
bug Something isn't working exporter/loadbalancing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[exporter/loadbalancing] Set a placeholder endpoint jpkrohling/opentelemetry-collector-contrib
3 participants
@jpkrohling @mrsimo @kurayama