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

Add metrics to snet #3257

Merged
merged 3 commits into from
Oct 21, 2019
Merged

Add metrics to snet #3257

merged 3 commits into from
Oct 21, 2019

Conversation

scrye
Copy link
Contributor

@scrye scrye commented Oct 16, 2019

Fixes #3107

Adds the following metrics:

# HELP lib_snet_closes_total Total number of Close calls.
# TYPE lib_snet_closes_total counter
lib_snet_closes_total 0
# HELP lib_snet_dials_total Total number of Dial calls.
# TYPE lib_snet_dials_total counter
lib_snet_dials_total 0
# HELP lib_snet_dispatcher_error_total Total number of dispatcher errors
# TYPE lib_snet_dispatcher_error_total counter
lib_snet_dispatcher_error_total 0
# HELP lib_snet_listens_total Total number of Listen calls.
# TYPE lib_snet_listens_total counter
lib_snet_listens_total 2
# HELP lib_snet_parse_error_total Total number of parse errors
# TYPE lib_snet_parse_error_total counter
lib_snet_parse_error_total 0
# HELP lib_snet_read_total_bytes Total number of bytes read
# TYPE lib_snet_read_total_bytes counter
lib_snet_read_total_bytes 1.163921e+06
# HELP lib_snet_read_total_pkts Total number of packetes read
# TYPE lib_snet_read_total_pkts counter
lib_snet_read_total_pkts 3105
# HELP lib_snet_scmp_error_total Total number of SCMP errors
# TYPE lib_snet_scmp_error_total counter
lib_snet_scmp_error_total 0
# HELP lib_snet_write_total_bytes Total number of bytes written
# TYPE lib_snet_write_total_bytes counter
lib_snet_write_total_bytes 579712
# HELP lib_snet_write_total_pkts Total number of packets written
# TYPE lib_snet_write_total_pkts counter
lib_snet_write_total_pkts 3037

This change is Reviewable

@scrye scrye added SNET feature New feature or request c/observability Metrics, logging, tracing labels Oct 16, 2019
@scrye scrye requested a review from lukedirtwalker October 16, 2019 14:37
@scrye scrye self-assigned this Oct 16, 2019
Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @scrye)


go/lib/snet/dispatcher.go, line 103 at r1 (raw file):

	hdr, ok := pkt.L4Header.(*scmp.Hdr)
	if !ok {
		metrics.M.SCMPErrors().Inc()

Hm I wonder if that's what kormat meant with SCMP errors? I thought maybe more something along the lines of SCMP errors received.

Copy link
Contributor Author

@scrye scrye left a comment

Choose a reason for hiding this comment

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

Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)


go/lib/snet/dispatcher.go, line 103 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

Hm I wonder if that's what kormat meant with SCMP errors? I thought maybe more something along the lines of SCMP errors received.

My approach was:

  • if no handler, consider the "unhandled SCMP" as an error;
  • handlers are free to then collect metrics for which SCMP parts they think are relevant;

I've changed the behavior in the PR a little bit right now:

  • calling the SCMP handler with a non-SCMP message is no longer considered an error;
  • if no handler found, assume the SCMP is an error (same as before), because it might very well be one;
  • the default handler increases the errors counter appropriately now;

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@scrye scrye merged commit acc86dd into scionproto:master Oct 21, 2019
@scrye scrye deleted the pubpr-fix-3107 branch October 21, 2019 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/observability Metrics, logging, tracing feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lib/snet: expose metrics
2 participants