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

Instrument producer_plugin for Prometheus #537

Merged
merged 11 commits into from
Dec 16, 2022

Conversation

ndcgundlach
Copy link
Contributor

Add a base set of metrics for to publish to Prometheus to the producer_plugin.

@@ -154,6 +154,7 @@ class chain_plugin_impl {
fc::microseconds abi_serializer_max_time_us;
std::optional<bfs::path> snapshot_path;

std::shared_ptr<chain_plugin_metrics> chain_metrics;
Copy link
Member

Choose a reason for hiding this comment

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

What necessitates this being a shared_ptr over a unique_ptr, or even just returning a reference to a const chain_plugin_metrics&?

@heifner
Copy link
Member

heifner commented Dec 7, 2022

See eosnetworkfoundation/mandel#67

plugins/net_plugin/include/eosio/net_plugin/net_plugin.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
struct runtime_metric {
metric_type type;
const std::string family;
const std::string label;
std::atomic<int64_t> value;
Copy link
Member

Choose a reason for hiding this comment

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

This should be initialized.

plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/prometheus_plugin/prometheus_plugin.cpp Outdated Show resolved Hide resolved
plugins/prometheus_plugin/prometheus_plugin.cpp Outdated Show resolved Hide resolved
@ndcgundlach ndcgundlach requested a review from heifner December 12, 2022 21:23

for (auto& rtm : _counters) {
auto new_val = static_cast<double>(std::get<2>(rtm).value);
std::get<1>(rtm).Increment(new_val-std::get<1>(rtm).Value());
Copy link
Member

Choose a reason for hiding this comment

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

Did you decide this was not needed?

@@ -832,6 +832,10 @@ class read_write {

} // namespace chain_apis

struct chain_plugin_metrics {
//runtime_metric
Copy link
Member

Choose a reason for hiding this comment

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

I assume this will be cleaned up with the chain_plugin PR to the feature branch.

Copy link
Contributor Author

@ndcgundlach ndcgundlach Dec 16, 2022

Choose a reason for hiding this comment

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

yes, changes for that are coming in the instrument chain plugin pr

@ndcgundlach ndcgundlach merged commit 2fb3e06 into feature_prometheus_plugin Dec 16, 2022
@ndcgundlach ndcgundlach deleted the instrument_producer_plugin branch December 16, 2022 16:07
cmadh pushed a commit to cmadh/leap that referenced this pull request Jan 2, 2023
…l-logging

[3.2] Additional transaction/block time logging
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