Skip to content

metrics: improve ux around _status/vars output#99516

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
dhartunian:status-vars-ux-improvements
Mar 28, 2023
Merged

metrics: improve ux around _status/vars output#99516
craig[bot] merged 1 commit intocockroachdb:masterfrom
dhartunian:status-vars-ux-improvements

Conversation

@dhartunian
Copy link
Copy Markdown
Collaborator

Previously, the addition of the tenant metric label was applied uniformly and could result in confusion for customers who never enable multi-tenancy or c2c. The tenant="system" label carries little meaning when there's no tenancy in use.

This change modifies the system tenant label application to only happen when a non- sytem in-process tenant is created.

Additionally, an environment variable:
COCKROACH_DISABLE_NODE_AND_TENANT_METRIC_LABELS can be set to false to disable the new tenant and node_id labels. This can be used on single-process tenants to disable the tenant label.

Resolves: #94668

Epic: CRDB-18798

Release note (ops change): The
COCKROACH_DISABLE_NODE_AND_TENANT_METRIC_LABELS env var can be used to disable the newly introduced metric labels in the _status/vars output if they conflict with a customer's scrape configuration.

@dhartunian dhartunian requested a review from a team March 24, 2023 19:57
@dhartunian dhartunian requested a review from a team as a code owner March 24, 2023 19:57
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@aadityasondhi aadityasondhi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @dhartunian)


pkg/server/status/recorder.go line 141 at r1 (raw file):

	mu struct {
		syncutil.RWMutex
		sync.Once

this is cool!

@dhartunian dhartunian force-pushed the status-vars-ux-improvements branch 3 times, most recently from 34c0056 to a529de1 Compare March 27, 2023 22:03
Previously, the addition of the `tenant` metric label was applied
uniformly and could result in confusion for customers who never enable
multi-tenancy or c2c. The `tenant="system"` label carries little
meaning when there's no tenancy in use.

This change modifies the system tenant label application to only
happen when a non- sytem in-process tenant is created.

Additionally, an environment variable:
`COCKROACH_DISABLE_NODE_AND_TENANT_METRIC_LABELS` can be set to
`false` to disable the new `tenant` and `node_id` labels. This can be
used on single-process tenants to disable the `tenant` label.

When the `tenantNameContainer` is nil, or the `nodeID` is set to
0, the labels will not be applied during recorder configuration on
startup. This is currently the case when running a separate process
tenant using `mt start-sql`. Those tenants *will not* have `tenant` or
`nodeID` labels available.

Resolves: cockroachdb#94668

Epic: CRDB-18798

Release note (ops change): The
`COCKROACH_DISABLE_NODE_AND_TENANT_METRIC_LABELS` env var can be used
to disable the newly introduced metric labels in the `_status/vars`
output if they conflict with a customer's scrape configuration.
@dhartunian dhartunian force-pushed the status-vars-ux-improvements branch from a529de1 to a6a1a4c Compare March 27, 2023 22:05
@dhartunian
Copy link
Copy Markdown
Collaborator Author

bors r=aadityasondhi

@dhartunian dhartunian added the backport-23.1.x PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only label Mar 28, 2023
@craig craig bot merged commit 0745cd4 into cockroachdb:master Mar 28, 2023
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 28, 2023

Build succeeded:

@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Mar 28, 2023

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


Backport to branch 23.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-23.1.x PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

metrics: improved UX for tenant labels on prometheus metrics

3 participants