cockroachdb / cockroachdb/cockroach

obs/ash: periodic OPS-log workload summary has no tenant attribution

Closed
#172,680 0 comments 0 reactions 0 assignees View on GitHub
A-multitenancy C-enhancement O-agent T-observability
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Is your feature request related to a problem? Please describe.**

The ASH sampler periodically emits a top-N workload summary to the OPS log channel as `AshWorkloadSummary` structured events (`obs.ash.log_interval`, default 10m). The sampler is a process-wide singleton, so in a shared-process multi-tenant deployment its ring buffer holds samples from all tenants, and `maybeLogSummary` aggregates across all of them.

The emitted event carries `work_event_type`, `work_event`, `workload_id`, and `sample_count`, but **no tenant ID** — even though every underlying `ASHSample` records a `TenantID`. As a result:

- The system tenant's OPS log mixes workloads from all in-process tenants with no way to attribute a summary entry to the tenant that produced it.
- Distinct workloads that happen to share a `(work_event_type, work_event, workload_id)` key across tenants are collapsed into a single aggregated entry, so counts are not per-tenant.

This is unlike the read paths (`ListLocalActiveSessionHistory`) and the persisted-ASH flusher, both of which correctly filter/attribute samples by `TenantID`. The periodic logging path is the one place tenant identity is dropped.

**Describe the solution you'd like**

Include tenant attribution in the periodic summary. Concretely:

- Add a `tenant_id` field to the `AshWorkloadSummary` event ([`pkg/util/log/eventpb/ash_events.proto`](https://github.com/cockroachdb/cockroach/blob/master/pkg/util/log/eventpb/ash_events.proto#L29)), and
- Extend the aggregation key in `maybeLogSummary` to include `TenantID`, so entries are ranked and reported per tenant ([`pkg/obs/ash/sampler.go`](https://github.com/cockroachdb/cockroach/blob/master/pkg/obs/ash/sampler.go#L382)).

`TenantID` is safe to log (non-sensitive), so it can be marked `redact:"nonsensitive"` like the other fields on the event.

**Describe alternatives you've considered**

Emit a separate summary per tenant rather than adding a field to the key. This is functionally equivalent but noisier and less aggregatable downstream; a `tenant_id` field is preferable.

**Additional context**

This gap only manifests in shared-process multi-tenancy; for separate-process tenants each pod has its own sampler and buffer, so the log is already single-tenant. Found while auditing ASH for multi-tenancy readiness. Note that `AshWorkloadSummary` is documented as "Reserved and subject to change without notice", so evolving it is low-risk.

Epic CRDB-64086

Jira issue: CRDB-65941

Contributor guide

Open the contributing guide

Research direction

Read the AshWorkloadSummary definition in pkg/util/log/eventpb/ash_events.proto and the aggregation in pkg/obs/ash/sampler.go, especially maybeLogSummary. Check the nearby tests and how the event fields are logged; done means summaries include tenant_id and aggregation and ranking distinguish tenants.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
observability
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.