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

metrics.go support for metadata queries(labels and series) #5971

Merged
merged 22 commits into from
May 4, 2022

Conversation

kavirajk
Copy link
Contributor

@kavirajk kavirajk commented Apr 20, 2022

What this PR does / why we need it:
We use metrics.go (a convention) to record some useful statistics about each of the instance and range queries.

So that we can query something like
{foo="bar"}|= "metrics.go" |logfmt|latency=slow

Where latency comes from statistics recorded in pkg/logql/metric.go.

This PR add support those experience for series and labels API as well.

Examples:

  1. Labels API (names)
curl -G -s  "http://127.0.0.1:3100/loki/api/v1/label"

Generates following log lines via metrics.go

level=info ts=2022-05-03T19:51:37.417252626Z caller=metrics.go:170 component=frontend org_id=145265 traceID=5bdfb2e229890ea2 latency=fast query_type=labels length=1h0m0s duration=65.140762ms status=200 label= throughput=0B total_bytes=0B total_entries=14
  1. Labels API (values)
curl -G -s  "http://127.0.0.1:3100/loki/api/v1/label/container/values"

Generates following log lines via metrics.go

level=info ts=2022-05-03T19:50:07.710590524Z caller=metrics.go:170 component=frontend org_id=145265 traceID=11349be1207cbdba latency=fast query_type=labels length=1h0m0s duration=11.239903ms status=200 label=container throughput=0B total_bytes=0B total_entries=19
  1. Series API
curl -G -s  "http://127.0.0.1:3100/loki/api/v1/series" --data-urlencode 'match[]={app="loki"}'

Generates following log lines via metrics.go

level=info ts=2022-05-03T19:51:40.272480868Z caller=metrics.go:215 component=frontend org_id=145265 traceID=396c8cdce34bb53e latency=fast query_type=series length=1h0m0s duration=85.968193ms status=200 match="{app=\"loki\"}" throughput=0B total_bytes=0B total_entries=249

Which issue(s) this PR fixes:
Fixes #4503

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

Future Work

  • Get rid of latency=slow|fast label from metric.go entries.
  • deprecate and remove returned_lines when query_type=filtered. Because now we have total_entries common for all the query_type.

Copy link
Contributor

@cstyan cstyan left a comment

Choose a reason for hiding this comment

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

Would we want any additional stats fields for these queries? the # of results for example

pkg/logql/metrics.go Outdated Show resolved Hide resolved
pkg/logql/metrics.go Outdated Show resolved Hide resolved
pkg/logql/metrics.go Outdated Show resolved Hide resolved
pkg/logql/metrics.go Show resolved Hide resolved
@kavirajk
Copy link
Contributor Author

Would we want any additional stats fields for these queries? the # of results for example

Good point. I think that would really be helpful. Let me try adding it.

Copy link
Collaborator

@trevorwhitney trevorwhitney left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@cstyan cstyan left a comment

Choose a reason for hiding this comment

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

LGTM as well, there's just a lint issue it looks like.

Thanks for implementing this Kavi. I learned a thing or two about how the stats collection is implemented while reviewing.

@kavirajk kavirajk marked this pull request as ready for review April 26, 2022 15:15
@kavirajk kavirajk requested a review from a team as a code owner April 26, 2022 15:15
@kavirajk
Copy link
Contributor Author

I'm facing one weird panic during some series API call. Investigating.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
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.

The approach is looking good. As you mentioned, we'll need to find/fix the panic and do some linting.

pkg/querier/queryrange/codec.go Outdated Show resolved Hide resolved
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
@kavirajk
Copy link
Contributor Author

kavirajk commented May 3, 2022

This is how the metrics.go entries looks like for following metadata queries.(tested it on some internal loki clusters)

  1. series
  2. label names
  3. label values (notice a value for label in metrics.go)

Screenshot_2022-05-03_21-55-25

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Comment on lines +186 to +187
// now assuming path has suffix `/values` label name should be second last to the suffix
// **if** there exists the second last.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we include a link to the docs for this endpoint? https://grafana.com/docs/loki/latest/api/#get-lokiapiv1labelnamevalues

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

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

@owen-d owen-d merged commit 1389857 into main May 4, 2022
@owen-d owen-d deleted the kavirajk/metrics.go-support-for-metadata-queries branch May 4, 2022 12:46
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.

We should log labels and series API in the frontend.
5 participants