Skip to content

Conversation

ADITYATIWARI342005
Copy link
Contributor

@ADITYATIWARI342005 ADITYATIWARI342005 commented Sep 25, 2025

What type of PR is this?

chore: CI & build tools improvement - enables promlinter to catch common Prometheus client usage issues

What this PR does / why we need it:

This PR enables the promlinter in golangci-lint to catch common Prometheus client usage issues and improve code quality for metrics-related code.

Changes made:

  • Linter Configuration: Updates tools/linter/golangci-lint/.golangci.yml to add promlinter with minimal settings (strict: false)

Impact:

  • CI will now report Prometheus metrics lint issues in future PRs
  • Local linter runs confirmed promlinter is active and integrated
  • Subsequent PRs modifying Prometheus metrics may need to address findings

Files Changed:

  • tools/linter/golangci-lint/.golangci.yml: Added promlinter configuration

Testing:

  • ✅ Local golangci-lint run performed
  • ✅ Promlinter confirmed active and integrated
  • ✅ No new failures from promlinter on current codebase

Which issue(s) this PR fixes:
#7054

Closes #7054

Release Notes: No

-enable promlinter in tools/linter/golangci-lint/.golangci.yml
-add brief DEVELOP.md note about Prometheus metrics linting
-no code changes required; CI will now report Prometheus metrics lint issues

Signed-off-by: ADITYATIWARI342005 <[email protected]>
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.98%. Comparing base (c4eed01) to head (a8d0620).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7063      +/-   ##
==========================================
- Coverage   71.02%   70.98%   -0.04%     
==========================================
  Files         227      227              
  Lines       40465    40465              
==========================================
- Hits        28741    28726      -15     
- Misses      10021    10034      +13     
- Partials     1703     1705       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Add promlinter to enabled linters in .golangci.yml
- Configure promlinter with strict: true
- No code changes required; CI will now report Prometheus metrics lint issues

Signed-off-by: ADITYATIWARI342005 <[email protected]>
@ADITYATIWARI342005 ADITYATIWARI342005 changed the title chore(lint): enable promlinter in golangci-lint and update docs chore(lint): enable promlinter in golangci-lint Sep 25, 2025
@arkodg
Copy link
Contributor

arkodg commented Sep 25, 2025

thanks @ADITYATIWARI342005, is there a simple way to prove that this linter is actually useful by for e.g. editing these metric names https://github.com/envoyproxy/gateway/blob/main/internal/message/metrics.go and seeing if the linter catches it ?

@ADITYATIWARI342005
Copy link
Contributor Author

@arkodg

Yes, the promlinter is proven useful,
I tested it by creating metrics with common violations in a test file:

  • counter metrics missing _total suffix
  • the linter immediately caught both violations
  • this will help in following Prometheus naming conventions automatically
image

@arkodg
Copy link
Contributor

arkodg commented Sep 26, 2025

@ADITYATIWARI342005 can your edit the above suggested file to see if it still performs similarly? We are using wrappers so want to make sure it works

@ADITYATIWARI342005
Copy link
Contributor Author

Ok I will test the changes and inform you in some time.

@ADITYATIWARI342005
Copy link
Contributor Author

ADITYATIWARI342005 commented Sep 26, 2025

@arkodg
Testing Results on Real Metrics Files:

  • Promlinter is working correctly - I tested it by adding violations to internal/message/metrics.go:
    Found:

  • Promlinter catches violations in direct Prometheus client usage (promauto.NewCounter)

  • ⚠️ Promlinter does NOT catch violations in the custom wrapper system (metrics.NewCounter)

image

Promlinter will catch issues if developers use direct Prometheus client calls.

@arkodg
Copy link
Contributor

arkodg commented Sep 27, 2025

thanks for testing this out @ADITYATIWARI342005, I dont think there's value in adding this linter then because we've chosen to use internal libs to generate metrics
cc @Xunzhuo

@ADITYATIWARI342005
Copy link
Contributor Author

@arkodg
Thank you for your review, & We may close issue #7054 then & this PR as well.

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.

Add support for promlinter in golangci-lint
2 participants