log.WithUserID: log user ID in user label instead of org_id

[dimitarvdimitrov]

Signed-off-by: Dimitar Dimitrov dimitar.dimitrov@grafana.com

What this PR does

• log.WithUserID: log user ID in user label instead of org_id
• updates the slow-queries dashboard to use the new label

This PR still keeps usages of __org_id__ in the TSDB.

Implications are that the slow queries dashboard will not detect old slow queries once this change is released.

This PR is complemented by grafana/dskit#156

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

• Tests updated
• [na] Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[pracucci]

Hold on before merging. We have some people working to expose selected logs to customers. It could impact them.

[pracucci]

@chroberts are you impacted by this change?

[colega]

Also, should we expose it on tenant label instead?

[chroberts]

@chroberts are you impacted by this change?

Thank you! Yes, this would impact us. I just added a change to an open PR to support user as well as org_id.

You can go ahead and merge whenever you'd like. Our side isn't complete yet, so we wouldn't be affected.

[pstibrany]

@colega raises a good point... we decided to try to use tenant terminology, right?

[pracucci]

@colega raises a good point... we decided to try to use tenant terminology, right?

As idealistic people, we did. The change would be quite wide spread, given almost all logs use user and probably none use tenant. Is that what we want?

[pstibrany]

@colega raises a good point... we decided to try to use tenant terminology, right?

As idealistic people, we did. The change would be quite wide spread, given almost all logs use user and probably none use tenant. Is that what we want?

I don't really mind using "user" in the logs, it's shorter :) But it's also extra rule to remember. ("user" for logs, but "tenant" in docs and arguments?) Perhaps it's time to finally unify all this terminology for once, and be done with it.

[colega]

I don't really mind using "user" in the logs, it's shorter :) But it's also extra rule to remember. ("user" for logs, but "tenant" in docs and arguments?) Perhaps it's time to finally unify all this terminology for once, and be done with it.

Totally agree with all of this.

Anyway, I checked and we don't seem to have any "tenant" in the logs, so I guess we could go with "user" now and replace usages in a separate issue.

[dimitarvdimitrov]

All metrics use user. So if we stick with user we'd at least have consistency between logs and metrics. Consistency with traces will come in grafana/dskit#156

What is the main concern with switching to tenant - that we might have some usages of user already or that the PR will be too big and it might be difficult to vet all places or something else?

I'm trying to think of an easy way to either log both or do the switch without a big change. We can replace the util_log.Logger to a logger that adds the tenant label when it sees a user label; or replaces the user label with a tenant label.

[pracucci]

All metrics use user. So if we stick with user we'd at least have consistency between logs and metrics. Consistency with traces will come in grafana/dskit#156

Makes sense.

What is the main concern with switching to tenant - that we might have some usages of user already or that the PR will be too big and it might be difficult to vet all places or something else?

It's definitely a big breaking change for metrics: I'm personally against doing that change in metrics, unless we'll have a smooth migration path, but the effort doesn't justify the benefit to me. Uniforming on user instead of tenant would be just easier, because we already use user almost everywhere (in both metrics and logs).

[pracucci]

@chroberts Mentioned they will handle this change and we can merge it, so this PR LGTM. About renaming from user to tenant, I left my personal opinion in the comment above.

[dimitarvdimitrov]

thanks for the reviews everyone. In this case I'm happy to merge this as is too