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

Metrics rename #346

Merged
merged 27 commits into from
Aug 23, 2021
Merged

Metrics rename #346

merged 27 commits into from
Aug 23, 2021

Conversation

eh-am
Copy link
Collaborator

@eh-am eh-am commented Aug 23, 2021

Refers to #332

Since this is a breaking change, I decided to clean up as much as possible.

  • Added pyroscope_ prefix.
  • Prefixed most metrics with _storage as they refer to the "storage" subsystem
  • Used histogram instead of gauge for durations
  • Added help messages
  • Updated the dashboard. Added a "Go" row that refers to the metrics exported by the prometheus client. Even though in certain environments it may be unnecessary (let's say there's already a dashboard for go metrics), it's still useful for us as a "batteries included" dashboard. It's collapsed by default, though.

Here's the result of the promlinter tool:

$ promlinter list . --add-position --add-help 

POSITION                           TYPE                NAME                                              HELP
benchmark/main.go:208:33           GAUGE               pyroscope_benchmark                               
benchmark/main.go:212:35           GAUGE               pyroscope_run_progress                            
benchmark/main.go:74:48            COUNTER             pyroscope_upload_errors                           
benchmark/main.go:78:53            COUNTER             pyroscope_successful_uploads                      
pkg/storage/storage.go:115:53      COUNTER             pyroscope_storage_writes_total                    number of calls to storage.Put
pkg/storage/storage.go:119:52      COUNTER             pyroscope_storage_reads_total                     number of calls to storage.Get
pkg/storage/storage.go:125:51      HISTOGRAM           pyroscope_storage_evictions_duration_seconds      duration of evictions (triggered when there's memory pressure)
pkg/storage/storage.go:132:52      GAUGE               pyroscope_storage_evictions_alloc_bytes           number of bytes allocated in the heap
pkg/storage/storage.go:136:52      GAUGE               pyroscope_storage_evictions_total_mem_bytes       total number of memory bytes
pkg/storage/storage.go:141:60      HISTOGRAM           pyroscope_storage_caches_flush_duration_seconds   duration of storage caches flush (triggered when server is closing)
pkg/storage/storage.go:146:60      HISTOGRAM           pyroscope_storage_db_close_duration_seconds       duration of db close (triggered when server is closing)
pkg/storage/storage.go:152:51      HISTOGRAM           pyroscope_storage_writeback_duration_seconds      duration of write-back writes (triggered periodically)
pkg/storage/storage.go:157:51      HISTOGRAM           pyroscope_storage_retention_duration_seconds      duration of old data deletion
pkg/storage/storage.go:192:56      COUNTER             pyroscope_storage_cache_hits_total                total number of cache hits
pkg/storage/storage.go:196:57      COUNTER             pyroscope_storage_cache_misses_total              total number of cache misses
pkg/storage/storage.go:200:64      COUNTER             pyroscope_storage_cache_reads_total               total number of cache queries
pkg/storage/storage.go:204:65      COUNTER             pyroscope_storage_cache_persisted_total           number of items persisted from cache to disk
pkg/util/debug/reporter.go:37:48   GAUGE               pyroscope_storage_disk_bytes                      size of items in disk
pkg/util/debug/reporter.go:41:53   GAUGE               pyroscope_storage_cache_size                      number of items in cache (memory)
pkg/util/debug/reporter.go:53:47   GAUGE               pyroscope_cpu_utilization                         cpu utilization (percentage)

eh-am added 27 commits August 12, 2021 19:23
remove all calls to metrics.Counter
and instead inline using the prometheus/promauto package
that's used to give more control and therefore
create unique registers per tests
avoiding the "duplicate metrics collector registration attempted" panic
message
see prometheus/client_golang#716 (comment)
couple things:
1 - semantics have changed, ie the metrics are initialized to 0
even if we can't ever generate metrics data

so think in a dashboard a 0 value vs simply nothing showing up

2 - some of the metrics generated were dynamic, for example,
based on files in a directory
that has been hardcoded
@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #346 (7c0056e) into main (f3186f4) will decrease coverage by 0.10%.
The diff coverage is 76.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #346      +/-   ##
==========================================
- Coverage   54.49%   54.40%   -0.09%     
==========================================
  Files         102      102              
  Lines        5020     4988      -32     
==========================================
- Hits         2735     2713      -22     
+ Misses       2035     2025      -10     
  Partials      250      250              
Impacted Files Coverage Δ
pkg/util/debug/reporter.go 0.00% <0.00%> (ø)
pkg/storage/periodic.go 75.48% <66.67%> (+0.04%) ⬆️
pkg/storage/cache/cache.go 56.87% <100.00%> (+0.99%) ⬆️
pkg/storage/storage.go 65.21% <100.00%> (-1.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3186f4...7c0056e. Read the comment docs.

Copy link
Member

@petethepig petethepig left a comment

Choose a reason for hiding this comment

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

Looks great, merging

@petethepig petethepig merged commit c22739d into grafana:main Aug 23, 2021
korniltsev pushed a commit that referenced this pull request Jul 18, 2023
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.

2 participants