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

collector-local timestamps for gnmi plugin #8411

Closed
rski opened this issue Nov 15, 2020 · 6 comments
Closed

collector-local timestamps for gnmi plugin #8411

rski opened this issue Nov 15, 2020 · 6 comments
Labels
area/gnmi feature request Requests for new plugin and for new features to existing plugins

Comments

@rski
Copy link

rski commented Nov 15, 2020

Feature Request

Proposal:

Timestamp gnmi updates using the collector's local time instead of using the timestamp field of the notification.

Current behavior:

Updates received over gnmi are always timestamped using the Timestamp field which comes fro mthe device.

Desired behavior:

Make it possible to use the collector's time when the update was received to timestamp updates.

Use case:

Depending on the device that sends gnmi updates, the timestamp semantics can be different. According to the spec, the process that streams gnmi notifications should use the time the modification happened, not the time it was seen by the process. For example, when the hardware counters change, the time when those counters changed is to be used.

This differs between implementations. Some devices stream a fresh timestamp every time they send an update to a client that is subscribed to a counter path, some use a fixed timestamp of the time the value came to be.

This results in two kinds of collected datum; One is a time series, where each timestamp from the update means counter was X at time T, with multiple values of T even if X did not change. In this case, you can find the current value by looking at the last update for the path which is at most the sample frequency. Looking at a window of T to T+(sample frequency) shows all the information about a device.
In the other, the timestamps could be at any point in the past, since they are the time at which a value was established. Looking at a window of T to T+(sample frequence) shows only the values that changed within that window.

Certain orgs prefer the first, spec breaking behaviour. This can be implemented on the collector in a device agnostic way by ignoring the timestamp provided by the device and recording the time an update was received.

I already have the code for this change, it's pretty simple to make it possible and configurable, but I thought I'd open an issue first to gauge the interest on this.

@rski rski added the feature request Requests for new plugin and for new features to existing plugins label Nov 15, 2020
@ssoroka
Copy link
Contributor

ssoroka commented Nov 16, 2020

I think we're fine with this change as long as it's a configuration option that needs to be enabled to take effect. If you'd like to open a PR we'd be happy to take a look at it

rski added a commit to rski/telegraf that referenced this issue Nov 17, 2020
Sometimes it can be useful to use a local, on receive timestamp for
gnmi notifications. This is the case for sample subscriptions. The
gnmi spec requires that the update timestamps are the time when the
value from the underlying data source (e.g. counter hardware) changed.
This value might not change for a long time, resulting in a timestamp
way back in the past. For such situations, it can be useful to use the
current time on the server, turning the semantics of the timestamp
into "This was the value on the device at time T according to the
collector" from "This was when the value last changed on the device."

Fixes influxdata#8411
rski added a commit to rski/telegraf that referenced this issue Nov 17, 2020
Sometimes it can be useful to use a local, on receive timestamp for
gnmi notifications. This is the case for sample subscriptions. The
gnmi spec requires that the update timestamps are the time when the
value from the underlying data source (e.g. counter hardware) changed.
This value might not change for a long time, resulting in a timestamp
way back in the past. For such situations, it can be useful to use the
current time on the server, turning the semantics of the timestamp
into "This was the value on the device at time T according to the
collector" from "This was when the value last changed on the device."

Fixes influxdata#8411
rski added a commit to rski/telegraf that referenced this issue Nov 17, 2020
Sometimes it can be useful to use a local, on receive timestamp for
gnmi notifications. This is the case for sample subscriptions. The
gnmi spec requires that the update timestamps are the time when the
value from the underlying data source (e.g. counter hardware) changed.
This value might not change for a long time, resulting in a timestamp
way back in the past. For such situations, it can be useful to use the
current time on the server, turning the semantics of the timestamp
into "This was the value on the device at time T according to the
collector" from "This was when the value last changed on the device."

Fixes influxdata#8411
rski added a commit to rski/telegraf that referenced this issue Nov 23, 2020
Sometimes it can be useful to use a local, on receive timestamp for
gnmi notifications. This is the case for sample subscriptions. The
gnmi spec requires that the update timestamps are the time when the
value from the underlying data source (e.g. counter hardware) changed.
This value might not change for a long time, resulting in a timestamp
way back in the past. For such situations, it can be useful to use the
current time on the server, turning the semantics of the timestamp
into "This was the value on the device at time T according to the
collector" from "This was when the value last changed on the device."

Fixes influxdata#8411
rski added a commit to rski/telegraf that referenced this issue Dec 7, 2020
Sometimes it can be useful to use a local, on receive timestamp for
gnmi notifications. This is the case for sample subscriptions. The
gnmi spec requires that the update timestamps are the time when the
value from the underlying data source (e.g. counter hardware) changed.
This value might not change for a long time, resulting in a timestamp
way back in the past. For such situations, it can be useful to use the
current time on the server, turning the semantics of the timestamp
into "This was the value on the device at time T according to the
collector" from "This was when the value last changed on the device."

Fixes influxdata#8411
@flyerhawk
Copy link

Was this implemented? Is there documentation? Sorry if this is a dumb question but I'm a Github noob networking engineer.

@ksator
Copy link

ksator commented Jan 6, 2021

@flyerhawk there is this pending pull request #8420

I just tested it using Dockerfile:

FROM golang:1.15.6-buster
WORKDIR /go/src
RUN git clone https://github.com/rski/telegraf.git
RUN cd /go/src/telegraf && make
RUN cp /go/src/telegraf/telegraf ../../bin/.

We also need to configure this feature (disabled by default) in the conf file use_local_timestamp = true

@sjwang90
Copy link
Contributor

Hello! We found that having your timestamps set locally would be very useful across a lot of Telegraf plugins, not just gnmi. Instead of building this enhancement for gnmi specifically we created an issue to create a global tag setting to set your local timestamp #8689.

We'd welcome any PRs for #8689.

If you are looking to use this immediately for gnmi, please use the list of builds from @rski's PR #8420 (comment). Otherwise the global tag feature will be on our roadmap to add soon.

rski added a commit to rski/telegraf that referenced this issue Mar 23, 2021
Sometimes it can be useful to use a local, on receive timestamp for
gnmi notifications. This is the case for sample subscriptions. The
gnmi spec requires that the update timestamps are the time when the
value from the underlying data source (e.g. counter hardware) changed.
This value might not change for a long time, resulting in a timestamp
way back in the past. For such situations, it can be useful to use the
current time on the server, turning the semantics of the timestamp
into "This was the value on the device at time T according to the
collector" from "This was when the value last changed on the device."

Fixes influxdata#8411
@sjwang90
Copy link
Contributor

The solution for this will be to utilize the Starlark processor. There is this example script that can be added to use the current/local timestamp over the field device's timestamp.

@rski
Copy link
Author

rski commented Apr 20, 2021

wow that's pretty awesome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gnmi feature request Requests for new plugin and for new features to existing plugins
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants