cockroachdb / cockroachdb/cockroach
obs/ash: periodic OPS-log workload summary has no tenant attribution
- 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
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