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

[wip] add ability to configure prometheus export via readers #7871

Merged
merged 10 commits into from
Jul 14, 2023

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented Jun 9, 2023

This enables end users to configure additional prometheus exporters for the collector's telemetry
via the readers configuration option. Configuring prometheus through the existing method
of setting the service::metrics::address will continue to work, and only log a warning for users
who have enabled the telemetry.useOtelWithSDKConfigurationForInternalTelemetry feature gate.

Linked issue: #7641

This is a follow up to #7679

@codeboten codeboten changed the title add ability to configure prometheus export via metric_readers [wip] add ability to configure prometheus export via metric_readers Jun 9, 2023
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Patch coverage: 85.82% and project coverage change: -0.08 ⚠️

Comparison is base (879fe77) 90.60% compared to head (8750281) 90.53%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7871      +/-   ##
==========================================
- Coverage   90.60%   90.53%   -0.08%     
==========================================
  Files         301      301              
  Lines       15209    15291      +82     
==========================================
+ Hits        13780    13843      +63     
- Misses       1148     1162      +14     
- Partials      281      286       +5     
Impacted Files Coverage Δ
internal/testutil/testutil.go 64.86% <57.14%> (-1.81%) ⬇️
service/telemetry.go 84.55% <82.75%> (+1.09%) ⬆️
service/internal/proctelemetry/config.go 96.70% <92.85%> (-3.30%) ⬇️
service/telemetry/config.go 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@codeboten codeboten force-pushed the codeboten/may-29-otlp-export branch 4 times, most recently from 2c8df44 to d8af4ab Compare June 22, 2023 21:04
@codeboten codeboten marked this pull request as ready for review June 22, 2023 22:30
@codeboten codeboten requested review from a team and mx-psi June 22, 2023 22:30
service/telemetry/config.go Outdated Show resolved Hide resolved
service/telemetry/config.go Outdated Show resolved Hide resolved
service/telemetry.go Outdated Show resolved Hide resolved
service/telemetry/config_test.go Outdated Show resolved Hide resolved
service/telemetry/config.go Outdated Show resolved Hide resolved
service/telemetry/config.go Outdated Show resolved Hide resolved
service/telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/config.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/config.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/config.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/config.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/config.go Outdated Show resolved Hide resolved
@codeboten codeboten force-pushed the codeboten/may-29-otlp-export branch from fa24823 to 72e9446 Compare June 23, 2023 18:22
@codeboten codeboten force-pushed the codeboten/may-29-otlp-export branch from 72e9446 to f8cc8c8 Compare July 7, 2023 21:21
@codeboten codeboten changed the title [wip] add ability to configure prometheus export via metric_readers [wip] add ability to configure prometheus export via readers Jul 10, 2023
service/telemetry.go Outdated Show resolved Hide resolved
service/telemetry/config.go Outdated Show resolved Hide resolved
@codeboten codeboten force-pushed the codeboten/may-29-otlp-export branch from 87232cb to b720440 Compare July 10, 2023 17:02
service/telemetry.go Outdated Show resolved Hide resolved
@codeboten codeboten force-pushed the codeboten/may-29-otlp-export branch 3 times, most recently from 912a4e9 to be90fe8 Compare July 13, 2023 19:18
Alex Boten and others added 6 commits July 13, 2023 14:04
Signed-off-by: Alex Boten <aboten@lightstep.com>
Signed-off-by: Alex Boten <aboten@lightstep.com>
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Signed-off-by: Alex Boten <aboten@lightstep.com>
Signed-off-by: Alex Boten <aboten@lightstep.com>
Signed-off-by: Alex Boten <aboten@lightstep.com>
Alex Boten added 3 commits July 13, 2023 14:04
Signed-off-by: Alex Boten <aboten@lightstep.com>
Signed-off-by: Alex Boten <aboten@lightstep.com>
Signed-off-by: Alex Boten <aboten@lightstep.com>
@codeboten codeboten force-pushed the codeboten/may-29-otlp-export branch from a329faf to 69ff4c3 Compare July 13, 2023 21:05
Signed-off-by: Alex Boten <aboten@lightstep.com>
@codeboten codeboten merged commit 9299728 into open-telemetry:main Jul 14, 2023
28 of 29 checks passed
@codeboten codeboten deleted the codeboten/may-29-otlp-export branch July 14, 2023 17:03
@github-actions github-actions bot added this to the next release milestone Jul 14, 2023
dmitryax pushed a commit that referenced this pull request Jun 28, 2024
…10343)

#### Description
Fixing the bug: the latest version of otel-collector failed to start
with ipv6 metrics endpoint service telemetry.

This problem began to occur after
#9037 with
the feature gate flag enabled was merged. This problem is probably an
implementation omission because the enabled codepath, which was
originally added by
#7871, is
marked as WIP.

You can reproduce the issue with the config and the environment variable
(`MY_POD_IP=::1`).
```yaml
service:
  telemetry:
    logs:
      encoding: json
    metrics:
      address: '[${env:MY_POD_IP}]:8888'
```

#### Link to tracking issue
Fixes
#10011

---------

Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
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