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

PMM-12155 Use new option name for node_exporter #2186

Merged
merged 16 commits into from
Nov 8, 2024

Conversation

0leksii
Copy link
Contributor

@0leksii 0leksii commented May 25, 2023

Since the new version of exporter-toolkit used in the node exporter the new option --web.config.file should be used instead to pass the TLS configuration.

PMM-12155

Link to the Feature Build: SUBMODULES-3284

Since the new version of exporter-toolkit used in the
node exporter the new option `--web.config.file` should be used
instead to pass the TLS configuration.
@0leksii 0leksii requested a review from a team as a code owner May 25, 2023 13:27
@0leksii 0leksii requested review from idoqo and PavelKhripkov and removed request for a team May 25, 2023 13:27
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 43.44%. Comparing base (9f00573) to head (399b8ec).
Report is 60 commits behind head on v3.

Files with missing lines Patch % Lines
managed/services/agents/mongodb.go 0.00% 0 Missing and 1 partial ⚠️
managed/services/agents/node.go 0.00% 0 Missing and 1 partial ⚠️
managed/services/agents/postgresql.go 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               v3    #2186      +/-   ##
==========================================
- Coverage   44.77%   43.44%   -1.33%     
==========================================
  Files         360      362       +2     
  Lines       36153    44100    +7947     
==========================================
+ Hits        16186    19158    +2972     
- Misses      18305    23259    +4954     
- Partials     1662     1683      +21     
Flag Coverage Δ
admin 11.49% <ø> (-0.05%) ⬇️
agent 51.80% <ø> (-1.22%) ⬇️
managed 45.21% <66.66%> (-1.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Improve variable declaration formatting.
managed/services/agents/agents.go Outdated Show resolved Hide resolved
managed/services/agents/node.go Outdated Show resolved Hide resolved
0leksii added 2 commits May 26, 2023 14:06
Add unit test to pass the new option to the exporter and
add some slight fixes to the formatting and styling.
@BupycHuk BupycHuk changed the base branch from main to v3 October 10, 2024 13:30
@BupycHuk BupycHuk requested review from ademidoff and JiriCtvrtka and removed request for PavelKhripkov October 10, 2024 23:17
@@ -30,7 +30,9 @@ import (
// The node exporter prior 2.28 use exporter_shared and gets basic auth config from env.
// Starting with pmm 2.28, the exporter uses Prometheus Web Toolkit and needs a config file
// with the basic auth users.
var v2_27_99 = version.MustParse("2.27.99")
var (
v2_28_00 = version.MustParse("2.28.0-0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Btw do you think we should eventually move this to features for consistency?

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to drop it eventually :)

version/features.go Outdated Show resolved Hide resolved
@BupycHuk BupycHuk merged commit 98bde60 into v3 Nov 8, 2024
28 of 32 checks passed
@BupycHuk BupycHuk deleted the PMM-12155-use-new-config-option branch November 8, 2024 11:07
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.

6 participants