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

Canary tweaks #2350

Merged
merged 7 commits into from
Jul 15, 2020
Merged

Canary tweaks #2350

merged 7 commits into from
Jul 15, 2020

Conversation

slim-bean
Copy link
Collaborator

A few follow up fixes after the PR yesterday mainly:

  • Adding note to upgrade guide about metric name changes
  • Instead of a metric recording the difference between the expected metrics result and actual result, expose two metrics one for actual and one for expected
  • Adjusted the locking to be a bit tighter around some methods
  • Added code to put a timeout on websocket read, as well as added a ping handler to increment a metric when the canary receives a websocket ping.
  • Added a check to make sure we are receiving messages from Loki at a regular interval, if not restart the websocket connection.

…gging, improve the mutex locking a bit, add a read timeout on the websocket, add a lastmessage time checker to restart websocket
change metric count reporting to two separate metrics, expected vs actual
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2020

Codecov Report

Merging #2350 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2350      +/-   ##
==========================================
+ Coverage   61.50%   61.52%   +0.02%     
==========================================
  Files         160      160              
  Lines       13536    13534       -2     
==========================================
+ Hits         8325     8327       +2     
+ Misses       4587     4583       -4     
  Partials      624      624              
Impacted Files Coverage Δ
pkg/canary/comparator/comparator.go 77.73% <100.00%> (+0.64%) ⬆️
pkg/promtail/targets/file/tailer.go 76.13% <0.00%> (ø)
pkg/querier/queryrange/downstreamer.go 97.93% <0.00%> (+2.06%) ⬆️

//If this entry equals or exceeds the spot check interval from the last entry in the spot check array, add it.
c.spotEntMtx.Lock()
Copy link
Member

Choose a reason for hiding this comment

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

very confused on the double lock/unlock instead of the single lock for this function, but I don't see it being a problem either.

Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

lgtm

# Conflicts:
#	docs/operations/upgrade.md
docs/operations/upgrade.md Outdated Show resolved Hide resolved
Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
@slim-bean slim-bean merged commit 2b145da into master Jul 15, 2020
@slim-bean slim-bean deleted the canary-tweaks branch July 15, 2020 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants