Skip to content

backport: hook up eventlog and eventlog metrics (#7981)#9510

Merged
sergio-mena merged 3 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-node
Oct 6, 2022
Merged

backport: hook up eventlog and eventlog metrics (#7981)#9510
sergio-mena merged 3 commits intotendermint:feature/adr075-backportfrom
mmsqe:feature/adr075-backport-node

Conversation

@mmsqe
Copy link

@mmsqe mmsqe commented Oct 5, 2022

For the full description, see #7981


PR checklist

  • Tests written/updated, or no tests needed
  • CHANGELOG_PENDING.md updated, or no changelog entry needed
  • Updated relevant documentation (docs/) and code comments, or no
    documentation updates needed

@mmsqe mmsqe marked this pull request as ready for review October 5, 2022 01:35
@mmsqe mmsqe requested a review from a team October 5, 2022 01:35
node/node.go Outdated
proxy.PrometheusMetrics(config.Namespace, "chain_id", chainID)
}
return cs.NopMetrics(), p2p.NopMetrics(), mempl.NopMetrics(), sm.NopMetrics(), proxy.NopMetrics()
return cs.NopMetrics(), nil, p2p.NopMetrics(), mempl.NopMetrics(), sm.NopMetrics(), proxy.NopMetrics()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there not a eventlog.NopMetrics()? I think this will panic if we leave it as nil

Copy link
Author

Choose a reason for hiding this comment

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

We have check nil here, but I added NopMetrics to be consistent, same default value.

}
}

func NopMetrics() *Metrics {
Copy link
Contributor

@sergio-mena sergio-mena Oct 5, 2022

Choose a reason for hiding this comment

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

The other NopMetrics seem to be generated code.
Is there a reason for this code to be manual?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, since other metrics generated after evetlog get introduced.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks a lot, @mmsqe !

@sergio-mena sergio-mena merged commit c3cc94a into tendermint:feature/adr075-backport Oct 6, 2022
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
…ndermint#9510)

* node: hook up eventlog and eventlog metrics (tendermint#7981)

* align no metrics for event log

* make use of metricsgen

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
mmsqe added a commit to mmsqe/tendermint that referenced this pull request Dec 9, 2022
…ndermint#9510)

* node: hook up eventlog and eventlog metrics (tendermint#7981)

* align no metrics for event log

* make use of metricsgen

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
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