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

remove nil checks around uses of autographer.stats #934

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

jmhodges
Copy link
Contributor

@jmhodges jmhodges commented Jul 30, 2024

This removes the now unneeded checks to see if autographer.stats is
nil wherever we gather stats.

We also remove some nil checks in signer.StatsClient that are no longer
workable as it accepts the statds.ClientInterface type.

Some of the older code was checking for error returns that they probably
shouldn't (logging about metrics problems tends to clutter up code and
the alerting should already tell you you have a problem), but we leave
those alone for now.

Updates AUT-159

@jmhodges jmhodges force-pushed the statsd-null-object-remove-checks branch 2 times, most recently from 318268a to 56f1628 Compare July 31, 2024 02:10
@jmhodges jmhodges changed the title statsd null object remove checks remove nil checks around uses of autographer.stats Jul 31, 2024
@jmhodges jmhodges marked this pull request as ready for review July 31, 2024 04:13
@jmhodges jmhodges requested review from a team as code owners July 31, 2024 04:13
@jmhodges jmhodges requested review from oskirby and diox and removed request for a team July 31, 2024 04:13
@jmhodges jmhodges changed the base branch from main to statsd-null-object July 31, 2024 04:13
bhearsum
bhearsum previously approved these changes Jul 31, 2024
This removes the now unneeded checks to see if `autographer.stats` is
nil wherever we gather stats.

We also remove some nil checks in signer.StatsClient that are no longer
workable as it accepts the statds.ClientInterface type.

Some of the older code was checking for error returns that they probably
shouldn't (logging about metrics problems tends to clutter up code and
the alerting should already tell you you have a problem), but we leave
those alone for now.

Updates AUT-159
@jmhodges jmhodges force-pushed the statsd-null-object-remove-checks branch from 56f1628 to 3bffa70 Compare August 1, 2024 02:04
@jmhodges jmhodges changed the base branch from statsd-null-object to main August 1, 2024 02:04
@jmhodges jmhodges dismissed bhearsum’s stale review August 1, 2024 02:04

The base branch was changed.

@jmhodges
Copy link
Contributor Author

jmhodges commented Aug 1, 2024

Ugh, when you change the base branch, it dismisses reviews. Sorry.

@jmhodges jmhodges requested a review from bhearsum August 1, 2024 02:31
@jmhodges jmhodges merged commit e701c90 into main Aug 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants