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.

Sign up for GitHub

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

Jump to bottom

Add query-frontend option to select request headers in query logs #11499

Merged
merged 12 commits into from
Jan 22, 2024
Merged

Add query-frontend option to select request headers in query logs #11499

merged 12 commits into from
Jan 22, 2024

Conversation

jmichalek132
Copy link
Contributor

@jmichalek132 jmichalek132 commented Dec 15, 2023 •
edited
Loading

What this PR does / why we need it:

Adding feature present in mimir, specifically grafana/mimir#5030.

Adds a config option to the query-frontend to specify a list of request headers to include in query logs.

For example, setting -frontend.log-query-request-headers="X-Grafana-Org-Id" and sending a query with X-Grafana-Org-Id:1 results in query log lines that include header_x_grafana_org_id=1.

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

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 15, 2023 •
edited
Loading

Trivy scan found the following vulnerabilities:

  • HIGH, Target: docker.io/grafana/loki:main-599eed7 (alpine 3.18.4), Type: alpine openssl: Incorrect cipher key and IV length processing in libcrypto3 v3.1.3-r0. Fixed in v3.1.4-r0
  • HIGH, Target: docker.io/grafana/loki:main-599eed7 (alpine 3.18.4), Type: alpine openssl: Incorrect cipher key and IV length processing in libssl3 v3.1.3-r0. Fixed in v3.1.4-r0
    \nTo see more details on these vulnerabilities, and how/where to fix them, please run docker build -t grafana/loki:main-599eed7 -f cmd/loki/Dockerfile .
    trivy i grafana/loki:main-599eed7 on your branch. If these were not introduced by your PR, please considering fixing them in via a subsequent PR. Thanks!

@jmichalek132 jmichalek132 force-pushed the jm-chore-fronted-log-arbitrary-headers branch from 2dabec3 to bfb9da2 Compare December 15, 2023 12:43
@CLAassistant
Copy link

CLAassistant commented Dec 15, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@jmichalek132 jmichalek132 marked this pull request as ready for review December 15, 2023 12:59
@jmichalek132 jmichalek132 requested a review from a team as a code owner December 15, 2023 12:59
@jmichalek132
Copy link
Contributor Author

Not sure how to safely update docs.

@JStickler
Copy link
Contributor

Not sure how to safely update docs.

@jmichalek132 I'm not quite sure what you mean by "safely" update the docs? All docs updates are published to the "next" branch until the next Loki release is cut. Doc updates aren't published to the current release unless someone backports the PR. So any changes to docs won't published until the code changes are part of a release. You can either do the docs updates as part of this PR (my recommendation) or as a separate PR.

@jmichalek132
Copy link
Contributor Author

Not sure how to safely update docs.

@jmichalek132 I'm not quite sure what you mean by "safely" update the docs? All docs updates are published to the "next" branch until the next Loki release is cut. Doc updates aren't published to the current release unless someone backports the PR. So any changes to docs won't published until the code changes are part of a release. You can either do the docs updates as part of this PR (my recommendation) or as a separate PR.

Hi, sorry for the late reply, I meant if there is something in the makefile that would generate the the docs for flags from the code, or is it just maintained manually?

@JStickler
Copy link
Contributor

@jmichalek132 The configuration reference and Helm reference are the only docs that I know of that we're automatically generating, as far as I know we're not generating any docs from Go files.

@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Jan 4, 2024
@jmichalek132
Copy link
Contributor Author

@JStickler I updated the docs.

@JStickler
Copy link
Contributor

@jmichalek132 The configuration reference should be updated in docs/sources/configure/index.template. There's a note to that effect at line 12 of the docs/sources/configure/_index.md file, you must have missed it.

@jmichalek132
Copy link
Contributor Author

@jmichalek132 The configuration reference should be updated in docs/sources/configure/index.template. There's a note to that effect at line 12 of the docs/sources/configure/_index.md file, you must have missed it.

I haven't modified that file directly, I ran make doc, which updated the file.

@JStickler
Copy link
Contributor

JStickler commented Jan 8, 2024 •
edited
Loading

Looks like we've got a file conflict, and this build error message "Please format code by running 'make format' and committing the changes" that needs fixing.

@jmichalek132
Copy link
Contributor Author

Looks like we've got a file conflict, and this build error message "Please format code by running 'make format' and committing the changes" that needs fixing.

Fixed both.

@jmichalek132
Copy link
Contributor Author

fyi the conflict comes from changelog, and will happen every time someone else updates. it.

dannykopping
dannykopping approved these changes Jan 22, 2024
View reviewed changes
Copy link
Contributor

@dannykopping dannykopping left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Very cool, thank you for your contribution @jmichalek132 👍
Can you please fix up the CHANGELOG.md file and then I'll merge? I fixed it up.

@dannykopping dannykopping enabled auto-merge (squash) January 22, 2024 10:45
@dannykopping dannykopping merged commit 0ed536c into grafana:main Jan 22, 2024
8 checks passed
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
@jmichalek132 @rhnasc
Add query-frontend option to select request headers in query logs (gr…
1c30c76 
…afana#11499)

**What this PR does / why we need it**:

Adding feature present in mimir, specifically
grafana/mimir#5030.

Adds a config option to the query-frontend to specify a list of request
headers to include in query logs.

For example, setting
-frontend.log-query-request-headers="X-Grafana-Org-Id" and sending a
query with X-Grafana-Org-Id:1 results in query log lines that include
header_x_grafana_org_id=1.

**Which issue(s) this PR fixes**:
Fixes grafana#11422
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dannykopping dannykopping dannykopping approved these changes

Assignees
No one assigned
Labels
size/M type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Log value of arbitrary request headers in query frontend stats log
4 participants
@jmichalek132 @CLAassistant @JStickler @dannykopping