feat: Add prometheus.echo component for local metrics inspection#4105
feat: Add prometheus.echo component for local metrics inspection#4105dehaansa merged 25 commits intografana:mainfrom
prometheus.echo component for local metrics inspection#4105Conversation
|
Can this be refactored to use the upstream encoding(s)? For example the expfmt encoder from the prometheus/common package |
|
sure @dehaansa will use prometheus expfmt encoder instead of custom string formatting and standard dto.MetricFamily protocol buffer |
|
Sorry that this dropped off our radar - it looks good, but needs a docs page. |
|
added the doc @dehaansa can you review ? |
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
|
Thanks for reviewing and for the commit, @clayton-cornell. Could you review @dehaansa’s one more time? |
dehaansa
left a comment
There was a problem hiding this comment.
Sorry that this slipped off my radar - LGTM. Once CI is happy, we can merge.
|
@iamrajiv When you have a moment, can you please sign the CLA. It's a requirement for moving forward on your PR. :-) |
There was a problem hiding this comment.
Pull request overview
This PR adds a new prometheus.echo component that receives Prometheus metrics and writes them to stdout in a human-readable format for debugging and testing purposes. The component mirrors the existing loki.echo component functionality but for Prometheus metrics instead of logs.
Key changes:
- New
prometheus.echocomponent with configurable output format (text or OpenMetrics) - Complete test coverage including format validation and metrics processing
- Documentation following the existing component doc structure
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/component/prometheus/echo/echo.go | Core component implementation with storage.Appendable interface, format encoding, and metric family building |
| internal/component/prometheus/echo/echo_test.go | Comprehensive test suite covering component lifecycle, appender operations, and format outputs |
| internal/component/all/all.go | Component registration in the global import list |
| docs/sources/reference/components/prometheus/prometheus.echo.md | Complete component documentation with usage examples and compatibility info |
| CHANGELOG.md | Release notes entry documenting the new feature |
| for k := range a.samples { | ||
| delete(a.samples, k) | ||
| } | ||
| for k := range a.exemplars { | ||
| delete(a.exemplars, k) | ||
| } | ||
| for k := range a.histograms { | ||
| delete(a.histograms, k) | ||
| } | ||
| for k := range a.metadata { | ||
| delete(a.metadata, k) | ||
| } |
There was a problem hiding this comment.
Instead of iterating and deleting keys individually, consider reinitializing the maps with make() which is more efficient: a.samples = make(map[string]sample) etc.
| for k := range a.samples { | |
| delete(a.samples, k) | |
| } | |
| for k := range a.exemplars { | |
| delete(a.exemplars, k) | |
| } | |
| for k := range a.histograms { | |
| delete(a.histograms, k) | |
| } | |
| for k := range a.metadata { | |
| delete(a.metadata, k) | |
| } | |
| a.samples = make(map[string]sample) | |
| a.exemplars = make(map[string]seriesExemplar) | |
| a.histograms = make(map[string]seriesHistogram) | |
| a.metadata = make(map[string]metadata.Metadata) |
| require.NoError(t, err) | ||
| }() | ||
|
|
||
| time.Sleep(100 * time.Millisecond) |
There was a problem hiding this comment.
Using time.Sleep in tests creates flakiness and unnecessarily slows down test execution. Consider using synchronization primitives or channels to properly wait for the component to start, or remove this line if it's not essential for the test.
docs/sources/reference/components/prometheus/prometheus.echo.md
Outdated
Show resolved
Hide resolved
|
@iamrajiv this should be ready, but it seems the CLA check has lost your info, can you resign the CLA? |
|
@dehaansa done can you check now |
The CLA Assistant seems to still think you haven't accepted: #4105 (comment) |
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…rafana#4862) * Introduce new dependency management tool * Move syntax replace to top of go.mod to avoid confusion * Apply replaces from tool * Introduce PR workflow to check modules are synced * Add E2E test * Address some PR comments Co-authored-by: Piotr <17101802+thampiotr@users.noreply.github.com> * Refactor to use a centralised cobra CLI, called by generate.go * Refactor to not generate .txt files and pass replace text in memory * Refactor E2E tests to use explicit test data * Update tools/generate-module-dependencies/replaces-mod.tpl Co-authored-by: Kyle Eckhart <kgeckhart@users.noreply.github.com> * Return errors in FileHelper instead of using Fatalf * Address PR comments * go mod/mod changes * Reference actual calculated file to dependency yaml file in error logs * Group the generate-module-dependencies and Check for go.mod changes properly in workflow and use git diff instead so we can ignore whitespace changes * nit: a few improvements for clarity --------- Co-authored-by: Piotr <17101802+thampiotr@users.noreply.github.com> Co-authored-by: Kyle Eckhart <kgeckhart@users.noreply.github.com>
…ana#5010) * fix: add StoreBuilder to mimir.alerts.kubernetes eventProcessor Fixes grafana#4975 * Add integration and unit tests * Fix lint error * Add changelog entry --------- Co-authored-by: Paulin Todev <paulin.todev@gmail.com>
* Add some high level pipeline tests for prometheus * Remove labelstore interactions from the interceptor * Move back to uber atomic and fix flaky remote write test * One more thing to move back to uber atomic * Actually fix the flaky test and lints * Fix flaky test for real and add a comment to Fanout about its role in global labels * Put componentID back on fanout for now
* Improve docker's multiplexed long lines handling * Fix `dockerChunkWriter` buffer flushing and add test for it * Update `CHANGELOG.md` * Fix potential panic in `extractTsFromBytes` and `dockerChunkWriter` buffer allocation
PR Description
The component provides local inspection of Prometheus metrics by outputting them to stdout in a human-readable format, making debugging and testing much easier.
Which issue(s) this PR fixes
Fixes: #3738
Notes to the Reviewer
PR Checklist