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

Add test case for vector matches on the promql compliance #92

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alanprot
Copy link

@alanprot alanprot commented Oct 20, 2022

Adding test case to cover cases like thanos-io/thanos#5799

Signed-off-by: Alan Protasio <approtas@amazon.com>
@@ -52,6 +52,7 @@ test_cases:
- query: 'demo_memory_usage_bytes{instance!~".*:10000"}'
- query: 'demo_memory_usage_bytes{type="free", instance!="demo.promlabs.com:10000"}'
- query: '{type="free", instance!="demo.promlabs.com:10000"}'
- query: 'promhttp_metric_handler_requests_in_flight and ignoring(code) promhttp_metric_handler_requests_total'
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! A couple of points:

  • This should go somewhere in the binary operator test queries section:
    # Binary operators.
    - query: '1 * 2 + 4 / 6 - 10 % 2 ^ 2'
    - query: 'demo_num_cpus + (1 {{.compBinOp}} bool 2)'
    variant_args: ['compBinOp']
    - query: 'demo_memory_usage_bytes {{.binOp}} 1.2345'
    variant_args: ['binOp']
    - query: 'demo_memory_usage_bytes {{.compBinOp}} bool 1.2345'
    variant_args: ['compBinOp']
    - query: '1.2345 {{.compBinOp}} bool demo_memory_usage_bytes'
    variant_args: ['compBinOp']
    - query: '0.12345 {{.binOp}} demo_memory_usage_bytes'
    variant_args: ['binOp']
    - query: '(1 * 2 + 4 / 6 - (10%7)^2) {{.binOp}} demo_memory_usage_bytes'
    variant_args: ['binOp']
    - query: 'demo_memory_usage_bytes {{.binOp}} (1 * 2 + 4 / 6 - 10)'
    variant_args: ['binOp']
    # Check that vector-scalar binops set output timestamps correctly.
    - query: 'timestamp(demo_memory_usage_bytes * 1)'
    # Check that unary minus sets timestamps correctly.
    # TODO: Check this more systematically for every node type?
    - query: 'timestamp(-demo_memory_usage_bytes)'
    - query: 'demo_memory_usage_bytes {{.binOp}} on(instance, job, type) demo_memory_usage_bytes'
    variant_args: ['binOp']
    - query: 'sum by(instance, type) (demo_memory_usage_bytes) {{.binOp}} on(instance, type) group_left(job) demo_memory_usage_bytes'
    variant_args: ['binOp']
    - query: 'demo_memory_usage_bytes {{.compBinOp}} bool on(instance, job, type) demo_memory_usage_bytes'
    variant_args: ['compBinOp']
    # Check that __name__ is always dropped, even if it's part of the matching labels.
    - query: 'demo_memory_usage_bytes / on(instance, job, type, __name__) demo_memory_usage_bytes'
    - query: 'sum without(job) (demo_memory_usage_bytes) / on(instance, type) demo_memory_usage_bytes'
    - query: 'sum without(job) (demo_memory_usage_bytes) / on(instance, type) group_left demo_memory_usage_bytes'
    - query: 'sum without(job) (demo_memory_usage_bytes) / on(instance, type) group_left(job) demo_memory_usage_bytes'
    - query: 'demo_memory_usage_bytes / on(instance, job) group_left demo_num_cpus'
    - query: 'demo_memory_usage_bytes / on(instance, type, job, non_existent) demo_memory_usage_bytes'
    # TODO: Add non-explicit many-to-one / one-to-many that errors.
    # TODO: Add many-to-many match that errors.
  • That section is currently woefully insufficient. It hasn't been testing ignoring at all (only on), so maybe adding similar queries with ignoring there as the currently existing on ones could already uncover this issue as well.
  • Currently the testing setup only scrapes data from the demo service (https://github.com/juliusv/prometheus_demo_service), and new queries should stick to that as well. You can play with all the demo_ metrics here: https://demo.promlabs.com/graph.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks!

About the last point those metrics are in fact from the demo server as we can see below.

You mean that ideally we should use just metrics that starts with demo_?

Screen Shot 2022-10-20 at 8 58 03 AM

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, it does get emitted as part of the standard client library metrics from the demo job, but I'd prefer to stick to the metrics that are explicitly created by the demo service itself if we can. So far all other examples only query those demo_ metrics.

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