[receiver/apache] Add metrics for max workers, bytes per second, and requests per second#47223
Conversation
… requests per second Add three new metrics to the Apache receiver from the server-status?auto endpoint: - apache.workers.max: total worker slots derived from scoreboard length - apache.bytes_per_sec: average bytes served per second (BytesPerSec field) - apache.requests_per_sec: average requests served per second (ReqPerSec field) Closes open-telemetry#47061
| ### apache.bytes_per_sec | ||
|
|
||
| The average number of bytes served per second since the server was started. |
There was a problem hiding this comment.
What about making this apache.request.io or apache.connection.io based on https://opentelemetry.io/docs/specs/semconv/general/naming/#instrument-naming
There was a problem hiding this comment.
Um, its an lifetime average, not an instataneous I/O, also just IO sounds also vague. and its not bidirectional - its just server's outbound bytes.
maybe we can come up with a better name.
There was a problem hiding this comment.
What about apache.request.rate.io or even apache.request_rate.io, with my preference being the later.
This way we could in the future also capture incoming data rate.
Alternatively we could do apache.request_rate.io.transferred or if we are sure we would only ever want to capture recieved we could do apache.request_rate.io.recieved
There was a problem hiding this comment.
how about apache.connection.tx_bytes ?
There was a problem hiding this comment.
The bytes seems some redundant in the name & we then are back at the initial situation and it is not clear that it is the average.
I think it would be nice to have it with alongside ie same sub namespace as apache.request_rate.count.
That way all apache.request_rate.* metrics are describing the requests being processed per second. I still like having io as that way we could also opt to capture not just incoming but outgoing.
There was a problem hiding this comment.
After looking at how other receivers in this repo handle similar metrics, I think .io isn't the right fit here - in OTel conventions, .io implies bidirectional data flow with a direction attribute (e.g., system.network.io, iis.network.io), but Apache's BytesPerSec is outbound-only server throughput.
The established pattern for bytes/second rates across receivers is .throughput:
- vcenter.host.network.throughput (KiBy/s)
- mongodbatlas.disk.partition.throughput (By/s)
- splunk.indexer.throughput (By/s)
Since we already have apache.traffic as the cumulative byte counter, I'd propose apache.traffic.throughput - it clearly signals this is the rate form of the existing traffic metric.
What do you think?
| apache.requests_per_sec: | ||
| description: "ApacheRequestsPerSecMetricConfig provides config for the apache.requests_per_sec metric." |
There was a problem hiding this comment.
| apache.requests_per_sec: | |
| description: "ApacheRequestsPerSecMetricConfig provides config for the apache.requests_per_sec metric." | |
| apache.request.count.1sec: | |
| description: "ApacheRequestsPerSecMetricConfig provides config for the apache.request.count.1sec." |
That way it suits alongside the other request metrics and naming guidance.
There was a problem hiding this comment.
This one is also a life time average rate (Total Accesses / Uptime), The 1sec name would be misleading as it implies 1 second window count. maybe we can keep it apache.request.rate
There was a problem hiding this comment.
How about apache.request_rate.count to complement the other metric.
There was a problem hiding this comment.
that sounds good.
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Applies reviewer feedback to use singular noun and 'limit' suffix following opentelemetry semantic convention instrument naming guidelines. regenerated code via make generate and reordered golden files.
Summary
Resolves #47061
Adds three new metrics to the Apache receiver, all sourced from the standard
server-status?autoendpoint:apache.workers.max(gauge, int) — Total worker slots, derived fromlen(Scoreboard). Enables utilization calculation:apache.workers{state=busy} / apache.workers.max.apache.bytes_per_sec(gauge, double) — Average bytes served per second, read fromthe
BytesPerSecfield.apache.requests_per_sec(gauge, double) — Average requests served per second, readfrom the
ReqPerSecfield.All three metrics are set to
stability: developmentandenabled: true.Changes
metadata.yaml— Added 3 new metric definitionsscraper.go— Added parsing forBytesPerSec,ReqPerSec, and scoreboard lengthmake generateTest plan
go test ./...)make lint)make generate)make chlog-validate)go test -tags=integration ./...)