feat(telemetry): enable statsd and dogstatsd telemetry sinks#18646
Conversation
statd and dogstatsd telemetry sinksstatsd and dogstatsd telemetry sinks
WalkthroughWalkthroughThe changes involve enhancing the telemetry and metrics system to support different backends, specifically statsd and dogstatsd, in addition to the existing in-memory sink. New configuration options have been added to specify the metrics backend type, statsd server address, and Datadog hostname. The code now handles the initialization of the specified metrics sink and integrates it into the telemetry system. Additionally, there are updates to tests and configuration defaults to align with these new features. Changes
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to filter (4)
- go.mod
- go.sum
- simapp/go.mod
- simapp/go.sum
Files selected for processing (1)
- telemetry/metrics.go (6 hunks)
Additional comments: 1
telemetry/metrics.go (1)
- 206-206: Verify that the
DisplayMetricsmethod implementations can handlenilvalues forhttp.ResponseWriterandhttp.Request.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- server/config/toml.go (1 hunks)
Additional comments: 2
server/config/toml.go (2)
121-133: The configuration template has been correctly updated to include the new telemetry sink options for
statsdanddogstatsd. This aligns with the PR objectives and the AI-generated summaries.121-133: Verify that the new configuration options are accompanied by updates to the documentation and that there are tests covering these new features.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This reverts commit 52d7b96.
alexanderbez
left a comment
There was a problem hiding this comment.
utACK. Thanks!!! Left a minor comment. Also, let's get a small changelog entry in for the new support.
|
@alexanderbez addressed nits |
|
@alexanderbez are you cool to backport into |
Yeah, I don't see why not. This change is simple and purely additive. No objections from me. @tac0turtle? |
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- server/config/toml.go (1 hunks)
- telemetry/metrics.go (8 hunks)
Files skipped from review as they are similar to previous changes (1)
- server/config/toml.go
Additional comments: 2
CHANGELOG.md (1)
- 59-59: The changes in the
CHANGELOG.mdcorrectly reflect the updates made to the system, including the addition of telemetry features, updates tosdk.DecCoins, improvements to thex/govmodule, and enhancements to theclient.Context{}. The links to pull requests provide traceability for the changes.telemetry/metrics.go (1)
- 207-213: Please verify that passing
nilfor bothhttp.ResponseWriterand*http.RequesttoDisplayMetricsis safe and will not cause any issues in all implementations ofDisplayableSink.
|
@julienrbrt updated v0.50 app toml as well |
|
Can you run |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (42)
- client/v2/go.mod
- client/v2/go.sum
- tests/go.mod
- tests/go.sum
- tests/starship/tests/go.mod
- tests/starship/tests/go.sum
- tools/confix/data/v0.50-app.toml
- tools/confix/data/v0.51-app.toml
- x/accounts/go.mod
- x/accounts/go.sum
- x/auth/go.mod
- x/auth/go.sum
- x/authz/go.mod
- x/authz/go.sum
- x/bank/go.mod
- x/bank/go.sum
- x/circuit/go.mod
- x/circuit/go.sum
- x/distribution/go.mod
- x/distribution/go.sum
- x/evidence/go.mod
- x/evidence/go.sum
- x/feegrant/go.mod
- x/feegrant/go.sum
- x/gov/go.mod
- x/gov/go.sum
- x/group/go.mod
- x/group/go.sum
- x/mint/go.mod
- x/mint/go.sum
- x/nft/go.mod
- x/nft/go.sum
- x/params/go.mod
- x/params/go.sum
- x/protocolpool/go.mod
- x/protocolpool/go.sum
- x/slashing/go.mod
- x/slashing/go.sum
- x/staking/go.mod
- x/staking/go.sum
- x/upgrade/go.mod
- x/upgrade/go.sum
Files selected for processing (1)
- telemetry/metrics.go (8 hunks)
Additional comments: 9
telemetry/metrics.go (9)
4-14: The addition of the
net/httpimport aligns with the introduction of theDisplayableSinkinterface, which useshttp.ResponseWriterandhttp.Request.23-30: The new constants for different metric sink types (
MetricSinkInMem,MetricSinkStatsd,MetricSinkDogsStatsd) are added to support the configuration of various telemetry sinks.32-35: The
DisplayableSinkinterface is introduced to define a method for displaying metrics, which is a part of the telemetry system refactoring to support different metric sinks.63-79: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [37-77]
The
Configstruct is expanded with new fields (MetricsSink,StatsdAddr,DatadogHostname) to configure the metrics backend type and the addresses forstatsdanddogstatsd.
82-87: The
Metricsstruct has been refactored to include a genericsinkfield of typemetrics.MetricSink, replacing the specificmemSink. This change supports the use of different metric sinks.103-141: The
Newfunction is modified to support the instantiation of different metric sinks based on theMetricsSinkconfiguration, including in-memory,statsd, anddogstatsdsinks.180-184: The
gatherPrometheusfunction includes a check to ensure Prometheus metrics are enabled before attempting to gather them, which is part of the improved error handling.206-216: The
gatherGenericfunction is updated to use theDisplayableSinkinterface for displaying metrics, enabling a more flexible display of metrics.122-125: Please verify if the issues raised in previous comments regarding the
deferstatement within thecase "mem":block and thenilcheck forsinkbefore adding it to theFanoutSinkhave been addressed in the current code.Also applies to: 138-138
|
@tac0turtle are you good with this? seems like tests are just weird flakiness |
b782d7d to
9ef410e
Compare
|
@itsdevbear made the inmemsink the default on the switch case, let me know if youre fine with that. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to filter (4)
- x/mint/go.mod
- x/nft/go.mod
- x/slashing/go.mod
- x/staking/go.mod
Files selected for processing (3)
- server/config/config.go (1 hunks)
- telemetry/metrics.go (8 hunks)
- telemetry/metrics_test.go (2 hunks)
Additional comments: 2
server/config/config.go (1)
- 234-238: The addition of
MetricsSinkto theTelemetryconfiguration inDefaultConfigis consistent with the PR's objective to enablestatsdanddogstatsdtelemetry sinks. Ensure thattelemetry.MetricSinkInMemis the intended default value forMetricsSink.telemetry/metrics_test.go (1)
- 20-25: The addition of the
MetricsSinkfield to theConfigstruct in theTestMetrics_InMemfunction is consistent with the PR's objective to enhance telemetry features. This change allows the test to specify the in-memory sink for metrics.
617ee04 to
4905617
Compare
) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com> Co-authored-by: Marko <marko@baricevic.me> (cherry picked from commit 3ba1c5b) # Conflicts: # CHANGELOG.md # client/v2/go.mod # go.mod # simapp/go.mod # simapp/gomod2nix.toml # tests/go.mod # tests/starship/tests/go.mod # tests/starship/tests/go.sum # tools/confix/data/v0.51-app.toml # x/accounts/go.mod # x/accounts/go.sum # x/auth/go.mod # x/auth/go.sum # x/authz/go.mod # x/authz/go.sum # x/bank/go.mod # x/bank/go.sum # x/circuit/go.mod # x/distribution/go.mod # x/distribution/go.sum # x/evidence/go.mod # x/feegrant/go.mod # x/gov/go.mod # x/gov/go.sum # x/group/go.mod # x/group/go.sum # x/mint/go.mod # x/mint/go.sum # x/nft/go.mod # x/params/go.mod # x/params/go.sum # x/protocolpool/go.mod # x/protocolpool/go.sum # x/slashing/go.mod # x/slashing/go.sum # x/staking/go.mod # x/staking/go.sum # x/upgrade/go.mod
* feat: secp256k1 public key constant time (cosmos#18026) Signed-off-by: bizk <santiago.yanzon1999@gmail.com> * chore: Fixed changelog duplicated items (cosmos#18628) * adr: Un-Ordered Transaction Inclusion (cosmos#18553) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * docs: lint ADR-070 (cosmos#18634) * fix(baseapp)!: postHandler should run regardless of result (cosmos#18627) * docs: fix typos in adr-007-specialization-groups.md (cosmos#18635) * chore: alphabetize labels (cosmos#18640) * docs(x/circuit): add note on ante handler (cosmos#18637) Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * fix: telemetry metric label variable (cosmos#18643) * chore: typos fix (cosmos#18642) * refactor(store/v2): updates from integration (cosmos#18633) * build(deps): Bump actions/setup-go from 4 to 5 (cosmos#18647) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * feat(store/v2): snapshot manager (cosmos#18458) * chore(client/v2): fix typos in the README.md (cosmos#18657) * fix(baseapp): protocompat.go gogoproto.Merge does not work with custom types (cosmos#18654) Co-authored-by: unknown unknown <unknown@unknown> * chore: fix several minor typos (cosmos#18660) * chore(tools/confix/cmd): fix typo in view.go (cosmos#18659) * refactor(x/staking): check duplicate addresses in StakeAuthorization's params (cosmos#18655) * feat(accounts): use gogoproto API instead of protov2. (cosmos#18653) Co-authored-by: unknown unknown <unknown@unknown> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(store/commitment/iavl): honor tree.Remove error firstly (cosmos#18651) * build(deps): Bump actions/stale from 8 to 9 (cosmos#18656) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(docs): fix typos & wording in docs (cosmos#18667) * chore: fix several typos. (cosmos#18666) * feat(telemetry): enable `statsd` and `dogstatsd` telemetry sinks (cosmos#18646) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com> Co-authored-by: Marko <marko@baricevic.me> * feat(store/v2): add SetInitialVersion in SC (cosmos#18665) * feat(client/keys): support display discreetly for `keys add` (cosmos#18663) Co-authored-by: Julien Robert <julien@rbrt.fr> * ci: add misspell action (cosmos#18671) * chore: typos fix by misspell-fixer (cosmos#18683) Co-authored-by: github-merge-queue <github-merge-queue@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr> * chore: add v0.50.2 changelog to main (cosmos#18682) * build(deps): Bump github.com/jhump/protoreflect from 1.15.3 to 1.15.4 in /tests (cosmos#18678) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * refactor(bank): remove .String() calls (cosmos#18175) Co-authored-by: Facundo <facundomedica@gmail.com> * ci: use codespell instead of misspell-fixer (cosmos#18686) Co-authored-by: Marko <marbar3778@yahoo.com> * feat(gov): add proposal types and spam votes (cosmos#18532) * feat(accounts): use account number as state prefix for account state (cosmos#18664) Co-authored-by: unknown unknown <unknown@unknown> * chore: typos fixes by cosmos-sdk bot (cosmos#18689) Co-authored-by: github-merge-queue <github-merge-queue@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr> Co-authored-by: marbar3778 <marbar3778@yahoo.com> * feat(client/keys): support display discreetly for keys mnemonic (cosmos#18688) * refactor: remove panic usage in keeper methods (cosmos#18636) * ci: rename pr name in misspell job (cosmos#18693) Co-authored-by: Marko <marko@baricevic.me> * build(deps): Bump github.com/pelletier/go-toml/v2 from 2.1.0 to 2.1.1 in /tools/confix (cosmos#18702) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * feat(client/keys): support display discreetly for keys export (cosmos#18684) * feat(x/gov): better gov genesis validation (cosmos#18707) --------- Signed-off-by: bizk <santiago.yanzon1999@gmail.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Carlos Santiago Yanzon <27785807+bizk@users.noreply.github.com> Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Co-authored-by: Akaonetwo <107335783+Akare123@users.noreply.github.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Julien Robert <julien@rbrt.fr> Co-authored-by: dreamweaverxyz <153101746+dreamweaverxyz@users.noreply.github.com> Co-authored-by: Pioua <136521243+dzizazda@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: cool-developer <51834436+cool-develope@users.noreply.github.com> Co-authored-by: leonarddt05 <139609434+leonarddt05@users.noreply.github.com> Co-authored-by: testinginprod <98415576+testinginprod@users.noreply.github.com> Co-authored-by: unknown unknown <unknown@unknown> Co-authored-by: Sukey <35202440+sukey2008@users.noreply.github.com> Co-authored-by: axie <152680487+azukiboy@users.noreply.github.com> Co-authored-by: Luke Ma <867273263@qq.com> Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com> Co-authored-by: 0xn4de <109149873+0xn4de@users.noreply.github.com> Co-authored-by: hattizai <150505746+hattizai@users.noreply.github.com> Co-authored-by: Devon Bear <itsdevbear@berachain.com> Co-authored-by: Marko <marko@baricevic.me> Co-authored-by: Halimao <1065621723@qq.com> Co-authored-by: Cosmos SDK <113218068+github-prbot@users.noreply.github.com> Co-authored-by: github-merge-queue <github-merge-queue@users.noreply.github.com> Co-authored-by: Facundo <facundomedica@gmail.com> Co-authored-by: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com>
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Enhancements
x/govmodule for direct proposal queries.Configuration Changes
metrics-backend,statsd-addr, anddatadog-hostnamesettings to telemetry configuration.Bug Fixes
Documentation
Refactor
Tests
MetricsSinkconfiguration field.Chores
set -euo pipefail.