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

bump(metrics): upgrade otel to the latest version #1537

Merged
merged 6 commits into from
Feb 2, 2023

Conversation

vgonkivs
Copy link
Member

@vgonkivs vgonkivs commented Dec 22, 2022

Overview

Updating otel collector to the latest version

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@vgonkivs vgonkivs added kind:deps Pull requests that update a dependency file area:metrics Related to measuring/collecting node metrics labels Dec 22, 2022
@vgonkivs vgonkivs self-assigned this Dec 22, 2022
@vgonkivs vgonkivs marked this pull request as ready for review January 25, 2023 10:41
@vgonkivs vgonkivs requested a review from derrandz January 25, 2023 10:42
Copy link
Contributor

@derrandz derrandz left a comment

Choose a reason for hiding this comment

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

Tested in branch https://github.com/derrandz/celestia-node/tree/derrandz/test-optl-version-bump which has #1638 against celestiaorg/test-infra#89 and everything is working great 👍🏻
:shipit:

@vgonkivs vgonkivs requested a review from Bidon15 January 27, 2023 16:30
Copy link
Contributor

@derrandz derrandz left a comment

Choose a reason for hiding this comment

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

Tested in branch https://github.com/derrandz/celestia-node/tree/bump_otel_v_x_blackbox_metrics which has #1376 against celestiaorg/test-infra#89 and it seems that there is an issue with Histograms in particular.

Not sure if the API has to change as well, can you look into this @vgonkivs pls?

@codecov-commenter
Copy link

codecov-commenter commented Feb 1, 2023

Codecov Report

Merging #1537 (d7af76b) into main (484933e) will increase coverage by 0.05%.
The diff coverage is 10.00%.

@@            Coverage Diff             @@
##             main    #1537      +/-   ##
==========================================
+ Coverage   55.09%   55.14%   +0.05%     
==========================================
  Files         230      230              
  Lines       14999    14989      -10     
==========================================
+ Hits         8263     8266       +3     
+ Misses       5853     5841      -12     
+ Partials      883      882       -1     
Impacted Files Coverage Δ
cmd/flags_misc.go 48.59% <ø> (ø)
header/pb/extended_header.pb.go 36.50% <ø> (ø)
nodebuilder/settings.go 0.00% <0.00%> (ø)
libs/header/p2p/pb/header_request.pb.go 40.79% <25.00%> (ø)
share/ipld/get.go 93.38% <0.00%> (+1.23%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@derrandz
Copy link
Contributor

derrandz commented Feb 1, 2023

Tested in branch https://github.com/derrandz/celestia-node/tree/derrandz/test-optl-version-bump-x-bbm which has #1376 against celestiaorg/test-infra#89 and everything is working superb.

Here's a screenshot of the metrics collected (including uptime metrics).
(don't mind the NO DATA panels as p2p metrics were purposefully disabled)

Screen Shot 2023-02-01 at 22 37 01

Copy link
Contributor

@derrandz derrandz left a comment

Choose a reason for hiding this comment

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

💸 🚢 🧑‍💻

@vgonkivs vgonkivs enabled auto-merge (squash) February 2, 2023 14:04
@Wondertan Wondertan disabled auto-merge February 2, 2023 14:09
@Wondertan Wondertan merged commit ba337b1 into celestiaorg:main Feb 2, 2023
renaynay pushed a commit to renaynay/celestia-node that referenced this pull request Feb 3, 2023
Co-authored-by: derrandz <ouaghad.hamza@gmail.com>
Co-authored-by: Hlib Kanunnikov <hlibwondertan@gmail.com>
renaynay added a commit that referenced this pull request Feb 8, 2023
…totalSampled das metrics + totalSynced sync metrics (#1638)

## Overview

This PR introduces node uptime metrics + das total sampled headers
metrics to support calculating the uptime index proposed by mustafa on
the monitoring side.

This PR introduces a new module named `Telemetry` to support node
related telemetry. This module can also host all general telemetry and
observability that does not interest specific modules.

## Changes

- [x] Introduced uptime metrics for node under
`nodebuilder/node/uptime.go`
- [x] Introduced persistent uptime metrics using datastore to persist
node start time
- [x] Testing for uptime metrics persistence using the store
- [x] Unit testing for uptime metrics
- [x] Integration testing for uptime metrics
- [ ] e2e testing for uptime metrics

## Checklist


- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [x] Visual proof for any user facing features like CLI or
documentation updates
- [ ] Linked issues closed with keywords

## Blocked By

PR: #1537

---------

Co-authored-by: rene <41963722+renaynay@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Related to measuring/collecting node metrics kind:deps Pull requests that update a dependency file
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants