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

Documentation: Refine LogQL documentation #2279

Merged
merged 4 commits into from
Jun 30, 2020
Merged

Documentation: Refine LogQL documentation #2279

merged 4 commits into from
Jun 30, 2020

Conversation

Fra-nk
Copy link
Contributor

@Fra-nk Fra-nk commented Jun 30, 2020

What this PR does / why we need it:
Documentation on LogQL had a very "all over the place" formatting:
Vastly different line lengths, inconsistent style(s), minor errors, etc.

  • Not sure if 1 line = 1 sentence is the preferred format, but at least it's consistent now.
    Previously some parts had 60 or 80 line length + brakes, while others had much longer lines.
    I'm also happy to preserve the old format (and consistently have that) if that's what you prefer.
    Independent of a few re-formatting things, I think the rest of the changes are worth it:
  • bool statement in conjunction with comparison operators was hard to grasp without that formatting.
  • Rewording of a few points, to emphasize important parts.
  • Avoid using quoting to format LogQL code blocks.

Which issue(s) this PR fixes:

Special notes for your reviewer:
Have a look at the Markdown rendered version(s), i.e. https://github.com/Fra-nk/loki/blob/logql-documentation-update/docs/logql.md and https://github.com/grafana/loki/blob/master/docs/logql.md

Checklist

  • Documentation added
  • Tests updated - did not find any

@CLAassistant
Copy link

CLAassistant commented Jun 30, 2020

CLA assistant check
All committers have signed the CLA.

docs/logql.md Outdated
Comment on lines 254 to 268
Between two scalars, the bool modifier must be provided and these operators result in another scalar that is either 0 (false) or 1 (true), depending on the comparison result.
Between two scalars, these operators result in another scalar that is either 0 (false) or 1 (true), depending on the comparison result.
The `bool` modifier must **not** be provided.
Copy link
Contributor Author

@Fra-nk Fra-nk Jun 30, 2020

Choose a reason for hiding this comment

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

Not sure about these sentences: The original

the bool modifier must be provided

seemed wrong, so I suggest this wording.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I just checked and this should actually state that between two scalars, the bool modifier is ignored.

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2020

Codecov Report

Merging #2279 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2279      +/-   ##
==========================================
+ Coverage   62.44%   62.45%   +0.01%     
==========================================
  Files         158      158              
  Lines       12762    12762              
==========================================
+ Hits         7969     7971       +2     
  Misses       4177     4177              
+ Partials      616      614       -2     
Impacted Files Coverage Δ
pkg/querier/queryrange/downstreamer.go 95.87% <0.00%> (-2.07%) ⬇️
pkg/logql/evaluator.go 92.54% <0.00%> (+0.40%) ⬆️
pkg/promtail/targets/tailer.go 78.40% <0.00%> (+2.27%) ⬆️

docs/logql.md Outdated

`1 + 2 / 3` will still be `1 + ( 2 / 3 )`.

Precedence order is identically to [mathematical convention](https://en.wikipedia.org/wiki/Order_of_operations).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Precedence order is identically to [mathematical convention](https://en.wikipedia.org/wiki/Order_of_operations).
Precedence order is identical to [mathematical convention](https://en.wikipedia.org/wiki/Order_of_operations).

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.

This looks great, thanks!

A few small nits, but looks good.

@ghost
Copy link

ghost commented Jun 30, 2020

I realised operator precedence isn't restricted to arithmetic operators, so I moved it to a dedicated section (under binary operators).

@owen-d owen-d merged commit 4ce385f into grafana:master Jun 30, 2020
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 2021
…c users data (grafana#2279)

* cache invalidation using cache gen numbers to invalidate only specific users data

using a middleware, all cache keys are prefixed with a gen number.
gen numbers are specific to users and are incremented whenever a new request comes in or
request is processed i.e requested data got deleted.

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* not caching results in querier frontend when there is inconsistency in cache generation numbers

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* use delete requests table for storing cache gen numbers and other changes suggested from PR review

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* check results cache gen numbers directly from delete requests store in frontend.

compares gen numbers from response headers and store to decide whether we need to cache results or not.
results cache would eliminate headers before encoding values to cache.
query sharding to pass back all the headers for sharded queries.

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* avoid throwing error while failing to load cache gen numbers

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* fix merge conflicts

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* changes suggested from PR review

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* changes suggested from PR review

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* changes suggested from PR review

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
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.

4 participants