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

gauge/counter #15

Closed
boucadair opened this issue Mar 3, 2023 · 9 comments
Closed

gauge/counter #15

boucadair opened this issue Mar 3, 2023 · 9 comments
Labels
ready_to_close? WGLC comment received during WGLC yangdoctors

Comments

@boucadair
Copy link
Contributor

Please check when it makes sense to use a gauge rather than a counter. For example:

+--ro num-event-avg?     yang:counter32
@fno2010
Copy link
Member

fno2010 commented Mar 3, 2023

Good point. yang:gauge64 seems more reasonable.

@boucadair
Copy link
Contributor Author

Thanks.

BTW, is there any reason all the counters are 32? may be moved those to be 64.

@fno2010
Copy link
Member

fno2010 commented Mar 3, 2023

Initially, we just thought 32 might be enough for the current deployment. But I agree. 64 will be better. I also think we should use 64.

@fno2010 fno2010 closed this as completed in c022440 Mar 6, 2023
@boucadair boucadair reopened this Mar 13, 2023
@boucadair boucadair added the WGLC comment received during WGLC label Mar 13, 2023
@boucadair
Copy link
Contributor Author

Reopening as I think the type has to be reassessed for some other data nodes (e.g., res-mem-size).

If the value may increase/decrease, then gauge64 should be used.

@fno2010
Copy link
Member

fno2010 commented Mar 13, 2023

Yes, you are right. I will check all the statistics data nodes to fix their types.

I think the following nodes should use gauge64 at least:

  • memory utilization
  • number of living connections
  • statistics in the last time window

fno2010 added a commit that referenced this issue Mar 13, 2023
Move all statistics data nodes that may increase/decrease to gauge64.
(comments by issue #15)

e.g., memory utilization, number of living connections, statistics in
the last time window...

Signed-off-by: jensenzhang <jingxuan.n.zhang@gmail.com>
@fno2010
Copy link
Member

fno2010 commented Apr 11, 2023

Track suggestions from yangdoctors early review by Andy:

5 minute counters

  • Should be type gauge64, not counter64 because the value can go down.

    • num-total-last-req
    • num-total-last-succ
    • num-total-last-fail

size counters

  • Should be type uint64, not counter64 because the value is a size.

    • res-mem-size
    • res-enc-size

update event counters

  • It is not clear what the correct data type should be. Not counter64.

    • num-event-max
    • num-event-min
  • Are these leafs maintained over a 5 minute interval perhaps?
    Then they should be gauge64.

  • The procedure to determine min and max should be specified or referenced.

@billwuqin
Copy link

I suggest to set data type for num-event-max and num-even-min as uint32, we can find some example in RFC8532 and RFC8299.
Regarding time interval, I am wondering whether time-window-size is related to time interval?

fno2010 added a commit that referenced this issue Apr 11, 2023
- Update description of `num-event-{max,min,avg}` to clarify the
  procedure to determine their values (#15)
- Add `num-event-{total,max,min,avg}-last` for stats within the last
  time window

Signed-off-by: jensenzhang <jingxuan.n.zhang@gmail.com>
@fno2010
Copy link
Member

fno2010 commented Apr 11, 2023

uint should always be safe. But maybe gauge provides more semantics? I think either work.

About the time interval, yes, it is configured by time-window-size. But num-event-* shows the total value, not the value in a given time interval. We just added a collection of stats num-event-*-last to provide values in the last time interval.

@fno2010
Copy link
Member

fno2010 commented Apr 11, 2023

Changed type of num-total-last-* to gauge64:

leaf num-total-last-req {
type yang:gauge64;
description
"The total number of ALTO requests received by the server
within the last time window.";
}
leaf num-total-last-succ {
type yang:gauge64;
description
"The total number of successful responses sent by the ALTO
server within the last time window.";
}
leaf num-total-last-fail {
type yang:gauge64;
description
"The total number of failed responses sent by the ALTO
server within the last time window.";
}

Changed type of res-*-size to uint64:

leaf res-mem-size {
type uint64;
units "bytes";
description
"Memory size utilized by the information resource.";
}
leaf res-enc-size {
type uint64;
units "bytes";
description
"Size of JSON encoded data of the information resource.";
}

Changed type of num-event-* to gauge64 (Qin suggests to simply using uint32) and updated the descriptions:

leaf num-event-max {
when 'derived-from-or-self(../../alto:resource-type,'
+ '"alto:update")';
type yang:gauge64;
description
"The maximum value over the total number of update events sent
to each connected session. Assume there are 3 connected
sessions A, B, and C with 4, 3, and 2 update events sent to
them respectively, the value of this metric is 4. After a
while, if there is no new update event sent to any session,
but the session A is disconnected, then the value of this
metric is updated to 3.";
}
leaf num-event-min {
when 'derived-from-or-self(../../alto:resource-type,'
+ '"alto:update")';
type yang:gauge64;
description
"The minimum value over the total number of update events sent
to each connected session. The procedure is similar to
num-event-max.";
}
leaf num-event-avg {
when 'derived-from-or-self(../../alto:resource-type,'
+ '"alto:update")';
type yang:gauge64;
description
"The average value over the total number of update events sent
to each connected session. The procedure is similar to
num-event-max.";
}

num-event-*-last are added to provide stats within the last time window:

leaf num-event-total-last {
when 'derived-from-or-self(../../alto:resource-type,'
+ '"alto:update")';
type yang:gauge64;
description
"Total number of update events sent to all the connected
sessions within the last time window.";
}
leaf num-event-max-last {
when 'derived-from-or-self(../../alto:resource-type,'
+ '"alto:update")';
type yang:gauge64;
description
"The maximum value over the number of update events sent
to each connected session within the last time window. The
procedure is similar to num-event-max, but only count the
update events within the last time window.";
}
leaf num-event-min-last {
when 'derived-from-or-self(../../alto:resource-type,'
+ '"alto:update")';
type yang:gauge64;
description
"The minimal value over the number of update events sent
to each connected session within the last time window. The
procedure is similar to num-event-max, but only count the
update events within the last time window.";
}
leaf num-event-avg-last {
when 'derived-from-or-self(../../alto:resource-type,'
+ '"alto:update")';
type yang:gauge64;
description
"The average value over the number of update events sent
to each connected session within the last time window. The
procedure is similar to num-event-max, but only count the
update events within the last time window.";
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready_to_close? WGLC comment received during WGLC yangdoctors
Projects
None yet
Development

No branches or pull requests

3 participants