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

feat: new metrics from Torch #1

Merged
merged 4 commits into from
Aug 18, 2023
Merged

feat: new metrics from Torch #1

merged 4 commits into from
Aug 18, 2023

Conversation

tty47
Copy link

@tty47 tty47 commented Aug 11, 2023

hello team!

I've added a couple of new metrics to torch.

the results look like:

Screenshot 2023-08-11 at 16 01 00

You can find it: here

they are deployed in robusta-torch-1, the metrics have been tested and seem to work fine.

Thanks in advance! 🚀

Jose Ramon Mañes

closes: https://github.com/celestiaorg/devops/issues/468

Signed-off-by: Jose Ramon Mañes <[email protected]>
Signed-off-by: Jose Ramon Mañes <[email protected]>
@tty47 tty47 self-assigned this Aug 11, 2023
@tty47 tty47 requested review from sysrex, Bidon15 and smuu August 11, 2023 14:02
@tty47 tty47 added the enhancement New feature or request label Aug 11, 2023
Signed-off-by: Jose Ramon Mañes <[email protected]>
Signed-off-by: Jose Ramon Mañes <[email protected]>
Copy link
Member

@smuu smuu left a comment

Choose a reason for hiding this comment

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

You 🪨

attribute.String("service_name", serviceName),
attribute.String("block_height_1", blockHeight),
attribute.String("earliest_block_time", earliestBlockTime),
attribute.Int("days_running", CalculateDaysDifference(earliestBlockTime)),
Copy link
Member

Choose a reason for hiding this comment

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

[non-blocking]

Is it possible to set the timestamp when the chain was started, and calculate how many days it's running in the frontend?

To my knowledge it's inefficient for monitoring systems to save metrics with labels that change over time. If I remember correctly, it's because of Prometheus saves metrics.

Copy link
Author

Choose a reason for hiding this comment

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

that would be great, but for now, I think is not possible due to the Grafana transformation doesn't have this feature yet to add calculations to attributes
I tried it before and I couldn't find a way to do it...

Copy link
Member

Choose a reason for hiding this comment

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

Interesting. What is the reason for having the "days running" as an attribute and not as a metric value? Like a timestamp.

(I don't think we should change your code, as it works. I want to understand different methods)

Copy link
Member

Choose a reason for hiding this comment

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

I guess blockchain hash can't be a metric value, as it's a string, right?

Copy link
Author

Choose a reason for hiding this comment

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

Interesting. What is the reason for having the "days running" as an attribute and not as a metric value? Like a timestamp.

(I don't think we should change your code, as it works. I want to understand different methods)

do you mean to set the value of the metric to the days running?, I think it is more clear to use an attribute, because the metric is called earliest_block_time which contains more data, another option could be just to create a new metric called days_running(or whatever), and set the value to it 🤔 I don't discard this option, we can split this info in a different metrics

wdyt?

the change is tiny, I'll only need a good name for the metric 😌

Copy link
Author

Choose a reason for hiding this comment

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

I would say no, we are using the metric type: Float64ObservableGauge, and the blockchain hash is a string... that's why we are setting the value 1 to the metric (also to make it like activated)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation!

Copy link
Member

@Bidon15 Bidon15 left a comment

Choose a reason for hiding this comment

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

The dashboard field should have a name like Genesis Hash not just block hash

@tty47 tty47 requested a review from smuu August 16, 2023 08:37
@tty47 tty47 merged commit 76b8750 into main Aug 18, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants