Skip to content

Commit

Permalink
monotonic sampled counter nanos should remain int64_t (#88)
Browse files Browse the repository at this point in the history
These values are not expected to rollover and will not ever go as high as
the actual data values.
  • Loading branch information
copperlight authored Jun 4, 2024
1 parent 07ba1c1 commit 124587a
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 5 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ echo -e "t:server.requestLatency:0.042\nd:server.responseSizes:1024" | nc -u -w0
| `X` | Monotonic Counter with Millisecond Timestamps | The value is a monotonically increasing number, sampled at a specified number of milliseconds since the epoch. A minimum of two samples must be received in order for `spectatord` to calculate a delta value and report it to the backend. The value should be a `uint64` data type, and it will handle rollovers. <br><br> This is an experimental metric type that can be used to track monotonic sources that were sampled in the recent past, with the value normalized over the reported time period. <br><br> The timestamp in milliseconds since the epoch when the value was sampled must be included as a metric option: `X,1543160297100:monotonic.Source:42` |

The data type for all numbers except `C` and `X` is `double`. The `C` and `X` values are recorded as `uint64_t`, and
the calculated deltas are passed to the backend as `double`.
the calculated deltas are passed to the backend as `double`. Passing negative values for `uint64_t` data types will
cause the parsed string value to rollover.

### Metric Name and Tags

Expand Down
1 change: 0 additions & 1 deletion server/spectatord.cc
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,6 @@ auto Server::parse_line(const char* buffer) -> std::optional<std::string> {
break;
case 'X':
if (extra > 0) {
// TODO: do we need to get this to match uint64_t?
// extra is milliseconds since the epoch
auto nanos = extra * 1000 * 1000;
registry_->GetMonotonicSampled(measurement->id)
Expand Down
1 change: 0 additions & 1 deletion spectator/measurement.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ class Measurement {
// Measurement(id.withTag(...), value);
Measurement(Id&& error, double value_param) noexcept = delete;

// TODO: should this be updated to support uint64_t for `C` and `X`? I don't think so, because deltas go here.
const Id& id;
double value;

Expand Down
2 changes: 1 addition & 1 deletion spectator/monotonic_sampled.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ MonotonicSampled::MonotonicSampled(Id id) noexcept
ts_{0},
prev_ts_{0} {}

void MonotonicSampled::Set(uint64_t amount, uint64_t ts_nanos) noexcept {
void MonotonicSampled::Set(uint64_t amount, int64_t ts_nanos) noexcept {
Update();
absl::MutexLock lock(&mutex_);

Expand Down
2 changes: 1 addition & 1 deletion spectator/monotonic_sampled.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class MonotonicSampled : public Meter {
explicit MonotonicSampled(Id id) noexcept;
void Measure(Measurements* results) const noexcept;

void Set(uint64_t amount, uint64_t ts_nanos) noexcept;
void Set(uint64_t amount, int64_t ts_nanos) noexcept;
auto SampledRate() const noexcept -> double;

private:
Expand Down

0 comments on commit 124587a

Please sign in to comment.