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

Loki Canary: One more round of improvements to query for missing websocket entries up to max-wait #2369

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

slim-bean
Copy link
Collaborator

@slim-bean slim-bean commented Jul 17, 2020

With wait set to only 1 min when we miss a websocket entry we check for it after 1 minute but only check one time.

If promtail was delayed or another hiccup existed somewhere the entry could still make it but later.

This PR introduces max-wait where the canary will continue to check for missing entries up to this time before officially marking them lost.

There was also a repeating pattern of removing elements from a slice which I extracted into a dedicated function for better reuse

…but continue checking until `max-wait`

Refactored out a useful pruneList function
@codecov-commenter
Copy link

Codecov Report

Merging #2369 into master will increase coverage by 0.10%.
The diff coverage is 95.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2369      +/-   ##
==========================================
+ Coverage   61.46%   61.56%   +0.10%     
==========================================
  Files         160      160              
  Lines       13534    13544      +10     
==========================================
+ Hits         8318     8339      +21     
+ Misses       4589     4583       -6     
+ Partials      627      622       -5     
Impacted Files Coverage Δ
pkg/canary/comparator/comparator.go 80.24% <95.58%> (+2.51%) ⬆️
pkg/logql/evaluator.go 92.88% <0.00%> (+0.40%) ⬆️
pkg/promtail/targets/file/filetarget.go 70.48% <0.00%> (+1.80%) ⬆️
pkg/promtail/targets/file/tailer.go 76.13% <0.00%> (+2.27%) ⬆️

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@slim-bean slim-bean merged commit 76ceec4 into master Jul 17, 2020
@slim-bean slim-bean deleted the canary-improvements-part-3 branch July 17, 2020 15:30
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.

None yet

3 participants