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

Format mgr:5min and mgr:60min as YAML #1796

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kinkie
Copy link
Contributor

@kinkie kinkie commented Apr 25, 2024

Also create a Time::YamlDateTime formatter

@kinkie
Copy link
Contributor Author

kinkie commented Apr 25, 2024

Output after PR:

sample start time: 2024-04-25 22:58:18.33Z
sample end time: 2024-04-25 23:01:18.33Z
HTTP clientside statistics:
  requests per second: 52.28
  hits per second: 0.00
  errors per second: 0.39
  kbytes in per second: 6.06
  kbytes out per second: 42.78
  median service time secs: 0.10
  miss median service time secs: 0.10
  near miss median service time secs: 0.00
  near hit median service time secs: 0.00
  hit median service time secs: 0.00
HTTP serverside statistics:
  requests per second: 51.74
  errors per second: 0.00
  kbytes in per second: 39.25
  kbytes out per second: 10.76
  request per second: 51.74
  HTTP errors per second: 0.00
  HTTP kbytes in per second: 39.25
  HTTP kbytes out per second: 10.76
FTP serverside statistics:
  requests per second: 0.00
  errors per second: 0.00
  kbytes in per second: 0.00
  kbytes out per second: 0.00
other protocols serverside statistics:
  requests per second: 0.00
  errors per second: 0.00
  kbytes in per second: 0.00
  kbytes out per second: 0.00
ICP statistics:
  packets sent per second: 0.00
  packets received per second: 0.00
  queries sent per second: 0.00
  replies sent per second: 0.00
  queries received per second: 0.00
  replies received per second: 0.00
  replies queued per second: 0.00
  queries timed out per second: 0.00
  kbytes sent per second: 0.00
  kbytes received per second: 0.00
  query kbytes sent per second: 0.00
  reply kbytes sent per second: 0.00
  query kbytes received per second: 0.00
  reply kbytes received per second: 0.00
  query median service time secs: 0.00
  reply median service time secs: 0.00
DNS statistics:
  median service time seconds: 0.00
OS statistics:
  unlink requests per second: 0.00
  page faults per second: 0.44
  select loops per second: 160.23
  select FDs per second: 0.00
  average select FD period: 0.00
  median select FDs: 0.00
  swapouts per second: 0.00
  swapins per second: 0.00
  swap files cleaned per second: 0.00
  aborted requests per second: 0.39
Cacheability statistics:
  hit validation attempts per second: 0.00
  hit validation refusals due to locking per second: 0.00
  hit validation refusals due to zero size per second: 0.00
  hit validation refusals due to time limit per second: 0.00
Syscall statistics:
  disk open per second: 0.29
  disk close per second: 0.28
  disk reads per second: 0.00
  disk writes per second: 0.00
  disk seeks per second: 0.00
  disk unlinks per second: 0.00
  socket create per second: 47.50
  socket accepts per second: 52.30
  socket connects per second: 47.50
  socket binds per second: 0.01
  socket closes per second: 99.80
  socket reads per second: 104.83
  socket writes per second: 156.00
  socket recvfroms per second: 0.02
  socket sendtos per second: 0.01
CPU time seconds: 5.18
Wall time seconds: 180.01
CPU usage percent: 2.88

@rousskov rousskov self-requested a review April 26, 2024 21:06
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

This review is incomplete. Please apply applicable change requests from other pending YAML PRs to this one. Please stop posting YAML PRs until the current set lands.

} // namespace Time

inline auto &
operator<<(std::ostream &os, const Time::YamlDateTime &dt)
Copy link
Contributor

Choose a reason for hiding this comment

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

Foo::X operators should be declared in Foo. See commit 25ecffe.

Time::YamlDateTime::print(std::ostream &os) const
{
// need to add fractions and timezone on top of this
static const char *yaml_time_format = "%Y-%m-%d %H:%M:%S";
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not see why we should name this format, but if you insist on naming it, please do not make the variable static and modifiable (there is no reason for any of that).

Suggested change
static const char *yaml_time_format = "%Y-%m-%d %H:%M:%S";
const auto yaml_time_format = "%Y-%m-%d %H:%M:%S";

static const char *yaml_time_format = "%Y-%m-%d %H:%M:%S";
const auto tm = gmtime(&tv_.tv_sec);
os << std::put_time(tm, yaml_time_format);
os << '.' << std::setw(2) << (tv_.tv_usec / 10000) << 'Z';
Copy link
Contributor

Choose a reason for hiding this comment

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

Why limit time to two digits?? Debugging often requires higher precision. Squid does not use two digits in most contexts IIRC. The chosen format does not specify precision AFAICT:

(\.[0-9]*[1-9])? # (fraction)

Please use all available digits.


/// Output onto an ostream a yaml-formatted datetime string (UTC)
/// see https://yaml.org/type/timestamp.html
class YamlDateTime
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to put YamlDateTime inside Time namespace? The class name already has "Time". And Yaml. And Date. Why pick Time out of those three? The namespace itself is highly questionable. I would keep this class outside that namespace.

class YamlDateTime
{
public:
YamlDateTime(const struct timeval &tv) : tv_(tv) {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid implicit conversions by default:

Suggested change
YamlDateTime(const struct timeval &tv) : tv_(tv) {};
explicit YamlDateTime(const struct timeval &tv): tv_(tv) {};

@kinkie kinkie added S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) M-ignored-by-merge-bots https://github.com/measurement-factory/anubis/blob/master/README.md#pull-request-labels labels May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-ignored-by-merge-bots https://github.com/measurement-factory/anubis/blob/master/README.md#pull-request-labels S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants