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

Fix vector-scalar comparisons #2189

Merged
merged 3 commits into from
Jun 5, 2020
Merged

Fix vector-scalar comparisons #2189

merged 3 commits into from
Jun 5, 2020

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Jun 5, 2020

  • formats shards in downstreamers for better logging
  • Make vector-scalar comparisons filter by default, but not set the results to 0 or 1.
$ logcli query 'sum(rate({filename=~"/var/log/wifi.*"}[1m])) > 1'
http://localhost:3100/loki/api/v1/query_range?direction=BACKWARD&end=1591386888234918000&limit=30&query=sum%28rate%28%7Bfilename%3D~%22%2Fvar%2Flog%2Fwifi.%2A%22%7D%5B1m%5D%29%29+%3E+1&start=1591383288234918000
[
  {
    "metric": {

    },
    "values": [
      [
        1591384744.234,
        "1893.2666666666664"
      ],
      [
        1591384758.234,
        "1893.2666666666664"
      ],
      [
        1591384772.234,
        "1893.2666666666664"
      ],
      [
        1591384786.234,
        "1893.2666666666664"
      ],
      [
        1591384800.234,
        "1074.3166666666666"
      ]
    ]
  }
]

vs

$ logcli query 'sum(rate({filename=~"/var/log/wifi.*"}[1m])) > bool 1'
http://localhost:3100/loki/api/v1/query_range?direction=BACKWARD&end=1591386974813316000&limit=30&query=sum%28rate%28%7Bfilename%3D~%22%2Fvar%2Flog%2Fwifi.%2A%22%7D%5B1m%5D%29%29+%3E+bool+1&start=1591383374813316000
[
  {
    "metric": {

    },
    "values": [
      [
        1591384746.813,
        "1"
      ],
      [
        1591384760.813,
        "1"
      ],
      [
        1591384774.813,
        "1"
      ],
      [
        1591384788.813,
        "1"
      ]
    ]
  }
]

@owen-d owen-d changed the title Fix/sharding Fix vector-scalar comparisons Jun 5, 2020
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

@codecov-commenter
Copy link

Codecov Report

Merging #2189 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2189      +/-   ##
==========================================
- Coverage   61.97%   61.96%   -0.02%     
==========================================
  Files         154      154              
  Lines       12432    12432              
==========================================
- Hits         7705     7703       -2     
- Misses       4120     4121       +1     
- Partials      607      608       +1     
Impacted Files Coverage Δ
pkg/logql/evaluator.go 92.72% <100.00%> (+0.41%) ⬆️
pkg/querier/queryrange/downstreamer.go 95.87% <100.00%> (ø)
pkg/promtail/targets/tailer.go 76.13% <0.00%> (-2.28%) ⬇️
pkg/ingester/transfer.go 65.49% <0.00%> (-1.41%) ⬇️

@owen-d owen-d merged commit 61fc175 into grafana:master Jun 5, 2020
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