Skip to content

Implement basic metric with prometheus#1178

Open
t0kieu wants to merge 2 commits intofireice-uk:devfrom
t0kieu:dev
Open

Implement basic metric with prometheus#1178
t0kieu wants to merge 2 commits intofireice-uk:devfrom
t0kieu:dev

Conversation

@t0kieu
Copy link

@t0kieu t0kieu commented Mar 21, 2018

Hi all,
This PR implement basic measuring with Prometheus metrics technology.
If you enable metrics while building app, then metrics are available on host:port/promethes

  1. counter - total submitted result based on the type.
  2. gauge - difficulty
  3. gauge - hash rare total
  4. gauge - hash rate per thread with device type.

Example of output:

# TYPE xmrstak_submitted_results_total counter
xmrstak_submitted_results_total{type="rejected"} 0.000000
xmrstak_submitted_results_total{type="accepted"} 91.000000
# TYPE xmrstak_difficulty_bucket gauge
xmrstak_difficulty_bucket 150.000000
# TYPE xmrstak_pool_connection_ping_seconds gauge
xmrstak_pool_connection_ping_seconds{address="xxx:3333"} 42.000000
# TYPE xmrstak_hash_rate_bytes_total gauge
xmrstak_hash_rate_bytes_total{le="900"} Nan
xmrstak_hash_rate_bytes_total{le="10"} 252.892318
xmrstak_hash_rate_bytes_total{le="60"} Nan
# TYPE xmrstak_hash_rate_bytes gauge
xmrstak_hash_rate_bytes{device="cpu",le="10",thread_id="12"} 18.3234
xmrstak_hash_rate_bytes{device="cpu",le="60",thread_id="12"} 17.378082
xmrstak_hash_rate_bytes{device="cpu",le="900",thread_id="5"} 21.585160

// CC: @yorik

Copy link

@yorik yorik left a comment

Choose a reason for hiding this comment

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

Thank you! I left some comments, but the main question: why to export rates, when just total would be enough?

rsp = MHD_create_response_from_buffer(str.size(), (void*)str.c_str(), MHD_RESPMEM_MUST_COPY);
MHD_add_response_header(rsp, "Content-Type", "application/json; charset=utf-8");
}
else if(strcasecmp(url, "/prometheus") == 0)
Copy link

Choose a reason for hiding this comment

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

Why no to go with default "/metrics" ?

Copy link
Author

Choose a reason for hiding this comment

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

Because it's Prometheus's related format of output. I don't have problem with metric name too. Name can be changed.

Copy link

Choose a reason for hiding this comment

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

It would require less custom settings, so I think it's better, but this is minor.

sMetric.gPoolPing = &gauge_family_pool_ping.Add({{"address", "xxx:3333"}});

auto& gauge_family_hr_total = BuildGauge()
.Name("xmrstak_hash_rate_bytes_total")
Copy link

Choose a reason for hiding this comment

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

I would named it "xmrstak_hash_rate_total"

Copy link
Author

Choose a reason for hiding this comment

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

By Prometheus convention units should be in name.

Copy link

Choose a reason for hiding this comment

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

But unit isn't bytes in this case, it's hashes per second, or am I missing something?

sMetric.agHashRateTotal[2] = &gauge_family_hr_total.Add({{"le", "900"}});

auto& gauge_family_hr = BuildGauge()
.Name("xmrstak_hash_rate_bytes")
Copy link

Choose a reason for hiding this comment

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

I would remove _bytes from here the metric name. Also it would be nice to have device and thread_id labels here too.

Copy link
Author

Choose a reason for hiding this comment

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

Bytes represent units: https://prometheus.io/docs/practices/naming/. Labels already exits, look at 1348-1362 there are dynamically created.

Copy link

Choose a reason for hiding this comment

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

Missed that. Thanks!

@t0kieu
Copy link
Author

t0kieu commented Mar 21, 2018

This is optimization :-) If you want total hash rate and you have big amount of data, then query to Prometheus backed is more performed, instead of doing sum calculation on all data aggregated by thread...

  • xmrstak_hash_rate_bytes_total represents summary of all hash rates from all threads and all devices.
  • xmrstak_hash_rate_bytes represents specific hash rate for specific thread and device identified via labels.

@psychocrypt
Copy link
Collaborator

THX for the PR, I will check this PR after the next release. I need first to check what prometheus

@psychocrypt psychocrypt self-assigned this Mar 24, 2018
@DrStein99
Copy link

Hi. I am interested in this, and was wondering if there was a link or document anywhere that can help me set this up (outside of my own obvious searches by using google).

I believe it is referencing prometheus.h somewhere, and not sure if the data collection software has to be installed on the xmr-stak miner, or what steps I need to try and set it up.

@erikgajdos
Copy link

Hey, looks good, looking forward once it will be merged and released. Any ETA?

@t0kieu
Copy link
Author

t0kieu commented Mar 26, 2018

@DrStein99 Try that simple steps:

  1. make build xmr-stak with promethes support
  2. run with http server enabled for example on port 12000
  3. download https://prometheus.io/download/ & run it.
  4. use that simple configuration for probmethes prometheus.yml (adjust IP:PORT of your miner).
global:
  scrape_interval:     15s # By default, scrape targets every 15 seconds.
scrape_configs:
  # XMR-STAK application
  - job_name: xmr-stak-application
    metrics_path: "/prometheus"
    static_configs:
      - targets: ['192.168.1.2:12000']
        labels:
          instance: your-hostname.com
          group: intranet

After a few seconds, promethes will be periodically connect (in that case every 15s) to your xmr-stak miner and it will be collects measured data.

@Tualua
Copy link

Tualua commented Oct 12, 2019

Any change to merge it into a current version?

@psychocrypt
Copy link
Collaborator

psychocrypt commented Oct 12, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants