Skip to content
This repository has been archived by the owner on Dec 6, 2024. It is now read-only.

Semantic conventions for Uptime Monitoring #185

Closed
wants to merge 5 commits into from

Conversation

jsuereth
Copy link
Contributor

A proposal and guide.

uptime is reported as a gauge with the value of the number of seconds that the process has been up. This is written as a gauge because users want to know the actual value of the number of seconds since the last restart to satisfy the use cases above. Sums are not a good fit for these use cases because most metric backends tend to default cumulative monotonic sums to rate-calculations, and have overflow handling that is undesired for this use case.

Sums report a total value that has accumulated over a time window; it is valid, for instance, to subtract the current value of a cumulative sum and restart the start timestamp to now. (OpenTelemetry's Prometheus receiver does this, for instance.)
An intended use case of a sum is to produce a meaningful value when aggregating away labels using sum. Such aggregations are not meaningful in the above use cases.
Copy link

Choose a reason for hiding this comment

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

One thing implied here is that aggregating sums doesn't add together the seconds since last restart of all processes. Aggregating counters request time windows to be aligned. That alignment changes the value in the sum to be for the new time window which doesn't preserve the actual value of seconds since last restart as stated in the first paragraph of this section.


## Trade-offs and mitigations

The biggest tradeoff here is defining `uptime` metrics as non-montonic sums vs. either pure gauge or non-montonic sums. The fundmental question here is whether default sum-based aggregation is meaningful for this metric, in addition to the default-query-capabilities of common backends for cumulative sums. The proposal trades-off allowing an external observer to monitor uptime (with resets) in addition to common assumptions on querying rates for cumulative sums.

Choose a reason for hiding this comment

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

nit, small typo on non-montonic

Choose a reason for hiding this comment

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

also fundmental => fundamental

| ---------------------- | ---------------------------- | ----- | -----------------------------|
| *.uptime | Seconds since last restart | s | Asynchronous Gauge |
| *.health | Availability flag. | 1 | Asynchronous Gauge |
| *.restart_count | Number of restarts. | 1 | Asynchronous Counter |
Copy link
Member

Choose a reason for hiding this comment

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

There is already a restart_count attribute defined for resources: https://github.com/open-telemetry/opentelemetry-specification/blob/a25d5f03ab58ecf88c09f635df97d2328b5ba237/specification/resource/semantic_conventions/k8s.md#container
It would be useful to tell how these two are related (if they are).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From a metric standpoint, having restart_count as a resource attribute is really really bad. I'm suprised no one commented on that, but I think it should be dropped, as it violates resource identity and causes high cardinality.

The TL;DR: though is that restart_count is something you'd want to alert on, and for metric-systems that means it needs to be a metric. They'd likely be the same value, but one is a resource attribute the other is a metric data point.

Copy link
Member

Choose a reason for hiding this comment

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

From a metric standpoint, having restart_count as a resource attribute is really really bad. I'm suprised no one commented on that, but I think it should be dropped, as it violates resource identity and causes high cardinality.

It was discussed, see open-telemetry/opentelemetry-specification#1945 (review)

The conclusion was that for a k8s container in a pod it is an identifying attribute, not a metric.

Choose a reason for hiding this comment

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

The Kubernetes restart count is not actually part of a unique-across-time identifier:

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.21/#containerstatus-v1-core

The number of times the container has been restarted, currently based on the number of dead containers that have not yet been removed. Note that this is calculated from dead containers. But those containers are subject to garbage collection. This value will get capped at 5 by GC.

Arguably that means it's not even useful as a metric, but it's certainly not a unique identifier.

| Name | Description | Units | Instrument Type |
| ---------------------- | ---------------------------- | ----- | -----------------------------|
| *.uptime | Seconds since last restart | s | Asynchronous Gauge |
| *.health | Availability flag. | 1 | Asynchronous Gauge |
Copy link
Member

Choose a reason for hiding this comment

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

The paragraph above nicely tells the drawbacks of "health" as a metric. As oposed to that I like kubernetes's approach. The "liveliness" and "readyness" are easier to define precisely: "liveliness" means "I am alive, let me run" (and the opposite of that means "I am in trouble, need help, restart me, do something"), and "readyness" means "I can now accept a workload".
I have a hard time assigning a similarly precise meaning to the "Healthy" metric. What does "availability" mean?
Given this, should we perhaps avoid "heath" as a metric altogether and perhaps instead use "ready" and "lively" (or whatever the names) metrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is meant to be the readyness flag. We could split this into two with readyness being recommended, and liveliness being optional.

Choose a reason for hiding this comment

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

I like the naming of readyness (readiness?) and liveliness, but if we're going down the road of multiple types, there actually could be arbitrary numbers of health status. For example, a backend could serve internal and external clients, and be unready for external clients but ready for internal clients. Can we just call it readyness and say "add an attribute if there are multiple distinct ready states"?


| Name | Description | Units | Instrument Type |
| ---------------------- | ---------------------------- | ----- | -----------------------------|
| *.uptime | Seconds since last restart | s | Asynchronous Gauge |
Copy link
Member

Choose a reason for hiding this comment

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

The general recommendations appear to say that .time suffix needs a dot. Should this be *.up.time if we follow the recommendation?

Copy link
Member

Choose a reason for hiding this comment

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

It seems *.time is normally used for things that have additive property.


## Motivation

Why should we make this change? What new value would it bring? What use cases does it enable?
Copy link
Member

Choose a reason for hiding this comment

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

These are just question and not "motivation"

Copy link
Member

Choose a reason for hiding this comment

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

I think this just the copy of the template: https://github.com/open-telemetry/oteps/blob/main/0000-template.md
@jsuereth probably forgot to update this section :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did, will update later in the week (a bit overloaded today)

@jmacd
Copy link
Contributor

jmacd commented Nov 15, 2021

I'd like to consider an alternative not mentioned in this document, and I'm not sure where to propose it.

Instead of two metrics, "health" and "uptime", I propose a single non-monotonic Sum named "alive" with value 1. This data type requires that a start time be included with the measurement, unlike Gauge. The difference between the start time and the measurement time is the process uptime.

I have made this proposal already in connection with open-telemetry/opentelemetry-specification#1078, where I pointed out that we can implement service discovery in a push-based metrics system by joining this "alive" metric with information retrieved by service discovery.

@jsuereth
Copy link
Contributor Author

@jmacd Commented offline, but recording here for posterity.

Instead of two metrics, "health" and "uptime", I propose a single non-monotonic Sum named "alive" with value 1. This data type requires that a start time be included with the measurement, unlike Gauge. The difference between the start time and the measurement time is the process uptime.

From a pure collection standpoint, I like a lot of what this brings, however I think we need to take an end-to-end focus. Specficially: "Can I write a query / dashboard / alert to solve the stated use cases?"

AFAICT, with known backends/query languages (Prometheus, Graphite, etc.) it's hard to pull the data back out, specifically the "Seconds since start" value in PromQL. We should make sure we have an answer to that.

tigrannajaryan added a commit to tigrannajaryan/opamp-spec that referenced this pull request Nov 17, 2021
OpenTelemetry is now working on defining how the health status should
be reported via metrics: open-telemetry/oteps#185

I am removing it from here so that if the metric is added we can use that.
In the unlikely even the metric is not added we can think whether we really
need it as a field in OpAMP.
tigrannajaryan added a commit to open-telemetry/opamp-spec that referenced this pull request Nov 17, 2021
OpenTelemetry is now working on defining how the health status should
be reported via metrics: open-telemetry/oteps#185

I am removing it from here so that if the metric is added we can use that.
In the unlikely even the metric is not added we can think whether we really
need it as a field in OpAMP.
@tedsuo tedsuo added the triaged label Jan 30, 2023
@tedsuo
Copy link
Contributor

tedsuo commented Jan 30, 2023

@jsuereth how important/relevant is this OTEP? Please assign an appropriate priority, or close if it's old and we no longer need it.

@tomasmota
Copy link

What is the state of this? It is still not clear to me how to implement this in otel. I suppose uptime is ok, but the health metric as 1|0 makes it not so useful. Should I then just do uptime for both, and only update health if the checks succeed?

Is it not a common use-case that most services would need this in some way? Or are people just relying directly on kubernetes checks instead? I understand that metric such as ops/sec. are much better, but not all services are doing stuff all the time, so this is much needed for those.

I had made an issue on this but closed it expecting this might progress. open-telemetry/opentelemetry-specification#2923

@erasmas
Copy link

erasmas commented Jun 13, 2023

I'm also curious about the state of this proposal since I'm having the same use case as described in open-telemetry/opentelemetry-specification#2923

@tedsuo
Copy link
Contributor

tedsuo commented Jul 31, 2023

@jsuereth is this stale, or is semconv currently working on this?

@Manuelraa
Copy link

I would also be interested in this. A generic up metric for creation of generic uptime alerting would be awesome.
Especially having it in the standard itself and e.g. integrated to OpenTelemetry Collector.

@carlosalberto
Copy link
Contributor

OTEPs have been moved to the Specification repository. Please consider re-opening this PR against the new location. Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.