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

Docs: Add multi-tenant query curl examples #6530

Merged
merged 2 commits into from
Jun 30, 2022

Conversation

KMiller-Grafana
Copy link
Contributor

This PR expands the curl examples given for the query API endpoint to include specifying a single tenant, multiple tenants, and gives the equivalent curl example, but for a GEL cluster.

Reviewers:

  • Are the curl commands completely correct?
  • Are there other API endpoints that should also have examples that specify tenants?

Copy link
Contributor

@09jvilla 09jvilla left a comment

Choose a reason for hiding this comment

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

Left some thoughts.

docs/sources/api/_index.md Outdated Show resolved Hide resolved
@@ -209,37 +217,37 @@ $ curl -G -s "http://localhost:3100/loki/api/v1/query" --data-urlencode 'query=
}
}
```
Set the `x-scope-orgid` header,
as defined in [Grafana Loki Multi-Tenancy](../operations/multi-tenancy/),
to query specific tenants.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
to query specific tenants.
to specify to Loki which specific tenant you want to query.

docs/sources/api/_index.md Outdated Show resolved Hide resolved
docs/sources/api/_index.md Show resolved Hide resolved
--data-urlencode \
'query=sum(rate({job="varlogs"}[10m])) by (level)' | jq
```
The same Grafana Enterprise Logs example query against the three tenants
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The same Grafana Enterprise Logs example query against the three tenants
The same Grafana Enterprise Logs example query against the three tenants requires the use of Basic Authentication.

Copy link
Contributor

Choose a reason for hiding this comment

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

This documentation is for Loki OSS right? Do we reference GEL there? I don't know the process well enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, @jeschkies. In the short term, we're including a GEL example in the Loki docs, so folks know that it is different for GEL. Our GEL readers have to find their info on this page.

docs/sources/api/_index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jeschkies jeschkies left a comment

Choose a reason for hiding this comment

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

Thank you. This looks good to me 🙂

docs/sources/api/_index.md Outdated Show resolved Hide resolved
docs/sources/api/_index.md Outdated Show resolved Hide resolved
docs/sources/api/_index.md Outdated Show resolved Hide resolved
specify the tenant names separated by the pipe (`|`) character:

```bash
curl -H 'x-scope-orgid:Tenant1|Tenant2|Tenant3' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
curl -H 'x-scope-orgid:Tenant1|Tenant2|Tenant3' \
curl -H 'X-Scope-OrgID: Tenant1|Tenant2|Tenant3' \

--data-urlencode \
'query=sum(rate({job="varlogs"}[10m])) by (level)' | jq
```
The same Grafana Enterprise Logs example query against the three tenants
Copy link
Contributor

Choose a reason for hiding this comment

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

This documentation is for Loki OSS right? Do we reference GEL there? I don't know the process well enough.

docs/sources/api/_index.md Outdated Show resolved Hide resolved
docs/sources/api/_index.md Outdated Show resolved Hide resolved
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
-               loki	-0.6%

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@jeschkies jeschkies requested a review from 09jvilla June 30, 2022 11:55
Copy link
Contributor

@09jvilla 09jvilla left a comment

Choose a reason for hiding this comment

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

One nit but approving to unblock! LGTM!

@@ -210,36 +218,39 @@ $ curl -G -s "http://localhost:3100/loki/api/v1/query" --data-urlencode 'query=
}
```

If your cluster has
[Grafana Loki Multi-Tenancy](../operations/multi-tenancy/) enabled,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[Grafana Loki Multi-Tenancy](../operations/multi-tenancy/) enabled,
[multi-tenancy](../operations/multi-tenancy/) enabled,

Feels weird to say 'Grafana Loki Multi-Tenancy' - makes it feel like its some sort of trademarked thing when multi-tenant software is sort of a general and widespread concept.

You could change it to If your Grafana Loki cluster has multi tenancy enabled should you so choose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does feel stilted and weird. However, a style guideline is to make the link name the same as the web page title or subsection title that we're sending someone to. It helps readers know that they've landed at the link we intended for them. That web page is titled "Grafana Loki Multi-tenancy," so I used it as the link title.

I'm going to leave this for now, but happy to figure out a future change that rewords and still follows the style guideline.

@KMiller-Grafana KMiller-Grafana merged commit 92aa69b into grafana:main Jun 30, 2022
@KMiller-Grafana KMiller-Grafana deleted the docs/tenants-in-queries branch June 30, 2022 20:20
@KMiller-Grafana KMiller-Grafana added the backport release-2.6.x Tag a PR with this label to create a PR which cherry pics it into the release-2.6.x branch label Jun 30, 2022
grafanabot pushed a commit that referenced this pull request Jun 30, 2022
* Docs: Add multi-tenant query curl examples

* Revise per review suggestions.

(cherry picked from commit 92aa69b)
vlad-diachenko pushed a commit that referenced this pull request Jul 5, 2022
* Docs: Add multi-tenant query curl examples

* Revise per review suggestions.

(cherry picked from commit 92aa69b)

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>
@osg-grafana osg-grafana added type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories and removed area/docs labels Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-2.6.x Tag a PR with this label to create a PR which cherry pics it into the release-2.6.x branch size/M type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants