blob: 35531cb167a0a657684ef47fa1167463289ab235 [file] [view] [edit]
# Naming data sources in a TraceConfig
**Authors:** @sashwinbalaji
**Status:** Draft
**PR:** N/A
This proposes an optional `alias` field on `DataSourceConfig`, so that one entry
in a `TraceConfig` can be pointed at unambiguously.
[rfc28]: https://github.com/google/perfetto/discussions/6179
[rfc13]: https://github.com/google/perfetto/discussions/4109
## Problem
`DataSourceConfig.name` is the data source _type_: `linux.ftrace`,
`android.power`, `linux.process_stats`. It cannot double as a handle for one
config entry, because the same type can legitimately appear more than once in
the same config:
```protobuf
data_sources {
config {
name: "linux.process_stats"
target_buffer: 1
process_stats_config {
scan_all_processes_on_start: true
}
}
}
data_sources {
config {
name: "linux.process_stats"
target_buffer: 2
process_stats_config {
quirks: DISABLE_ON_DEMAND
proc_stats_poll_ms: 30000
record_process_dmabuf_rss: true
}
}
}
```
Two entries of the same type, different settings, different target buffers.
Nothing else in the config can point at one of them specifically.
Almost nothing in a config refers to a data source today, and the one thing that
does, `ChangeTraceConfig()`, already gets it wrong (see below).
[RFC-0028][rfc28] makes those references routine: a routing rule has to say
which data source it applies to, and against the config above, "the one called
`linux.process_stats`" is ambiguous.
The positional index works, but:
- it is unreadable once a config gets big
- it silently starts meaning something else when somebody adds or reorders an
entry above it
The consumer API has the same gap today, without waiting for any new config
field: a consumer watching those two entries start cannot tell one event from
the other. See Observable events below.
## Design
```protobuf
message DataSourceConfig {
optional string name = 1; // unchanged: the data source TYPE
// Optional handle for THIS entry in the TraceConfig, so other parts of the
// config can refer to this data source unambiguously even when several
// entries share the same |name|. Must be unique within a tracing session.
// Set by the consumer; never overwritten by the service.
// Introduced in v59.
optional string alias = 13;
}
```
`alias` is optional. An entry that doesn't set one cannot be referenced.
If it is set, it has to be unique within the tracing session. A duplicate is a
config error, rejected at `EnableTracing` with a message naming both offending
entries, and logged with a new `PerfettoStatsdAtom` value.
An empty string counts as unset.
The change is additive, so existing configs are unaffected. Older services
ignore the unknown field, so a config carrying an alias still runs on them, it
just can't be referenced there.
### Aliases are scoped to the session
[Buffer names][rfc13] already work this way. An alias resolves inside the config
that declares it and nowhere else, so two concurrent sessions using the same
alias for completely unrelated data sources is fine, and not something we try to
detect. (Why not global: see Alternatives.)
Which means that from a producer's point of view an alias is not unique, since
one producer can be in several sessions at once. That is fine too, because
nothing on the producer side keys off it:
```cpp
// include/perfetto/ext/tracing/core/producer.h
virtual void SetupDataSource(DataSourceInstanceID,
const DataSourceConfig&) = 0;
```
The key is the `DataSourceInstanceID` argument, which comes out of one
service-wide counter (`++last_data_source_instance_id_`) and is unique across
every session and producer. The alias travels with it as a label, and nothing on
the producer side ever looks it up.
### Reference fields resolve against aliases only
Whenever such fields arrive, they resolve against `alias` and never against
`name`.
That has two consequences. An alias that collides with a data source type name
needs no special handling, and an alias is allowed to equal another entry's
`name`. If that turns out to confuse people we can forbid it later, which is
much easier than the other way round.
### Where the field goes
On `DataSourceConfig`, not on the `TraceConfig.DataSource` wrapper around it.
```protobuf
data_sources { // TraceConfig.DataSource, the wrapper
config { // DataSourceConfig
name: "linux.sys_stats"
alias: "meminfo_fast" // <- proposed placement
target_buffer_name: "slow_sysstats"
}
producer_name_filter: "perfetto.traced_probes" // wrapper-level field
}
```
Two reasons:
- The wrapper never leaves the service. `SetupDataSource()` sends the producer
only the inner `config`, so that is where the alias has to live to reach the
producer's process, where an instance can log which config entry it came from.
- It sits next to `target_buffer_name`, which is the same kind of field: written
by the consumer, read by the service.
## Knock-on effects
Adding the field touches three things beyond the proto change. I cannot tell
how much any of them matters in practice, possibly because I am missing
historical context, so all three are noted here for completeness.
### Observable events
A consumer gets one of these per data source instance, and it has nowhere to put
a handle:
```protobuf
// observable_events.proto
message DataSourceInstanceStateChange {
optional string producer_name = 1;
optional string data_source_name = 2;
optional DataSourceInstanceState state = 3;
}
```
Instances come from config entries, so a consumer that enables both
`linux.process_stats` entries from the Problem section and then watches them
start gets two events with identical contents, and no way to work out which is
which. `TracingMuxerImpl::ConsumerImpl` shows what that costs in practice:
```cpp
// src/tracing/internal/tracing_muxer_impl.h
using DataSourceHandle = std::pair<std::string, std::string>;
std::map<DataSourceHandle, bool> data_source_states_;
```
The pair is `(producer_name, data_source_name)`, so our two entries collapse
onto a single map key.
The same ambiguity turns up in two places that write into the trace rather than
back to the consumer. Both list data sources by producer name and type:
- `slow_starting_data_sources`, emitted when a data source takes more than 20s
to start, alongside a matching `PERFETTO_LOG`
- `last_flush_slow_data_sources`, emitted when a flush times out
So somebody reading the trace back cannot tell which of the two
`linux.process_stats` entries was the slow one either.
### C++ SDK startup tracing
I am logging this rather than proposing we act on it now, because it lands
entirely on the C++ SDK, which is in maintenance.
It is also the only place where adding a field to `DataSourceConfig` costs
anything. `DataSourceBase::CanAdoptStartupSession()` decides whether an already
running startup tracing session can be handed over to the real session traced is
now setting up. It compares the two configs whole, after zeroing out the fields
the service is known to fill in:
```cpp
// src/tracing/data_source.cc
startup_config_stripped.set_target_buffer(0);
startup_config_stripped.set_tracing_session_id(0);
startup_config_stripped.set_session_initiator(
DataSourceConfig::SESSION_INITIATOR_UNSPECIFIED);
startup_config_stripped.set_trace_duration_ms(0);
startup_config_stripped.set_stop_timeout_ms(0);
startup_config_stripped.set_enable_extra_guardrails(false);
// ... and the same six on service_config_stripped ...
return startup_config_stripped == service_config_stripped;
```
The startup config is written by the app, the session config by the consumer, so
they have different authors by construction. Anything the consumer sets that the
app does not replicate exactly makes that comparison fail, and when it fails the
instance never gets bound and we lose the exact window startup tracing exists to
capture.
`alias` is consumer-set and reaches the producer untouched, so it has to join
that list:
```cpp
startup_config_stripped.set_alias("");
service_config_stripped.set_alias("");
```
While we are in there: `target_buffer_name` has the same shape and the same
problem. We added it in v54, the consumer sets it, it passes through to the
producer untouched, and it never made it into the strip list. So a consumer that
moves an entry from `target_buffer` to `target_buffer_name` today already breaks
adoption for any app whose startup config still spells the buffer as an index.
`TrackEventDataSource` overrides `CanAdoptStartupSession()` and compares only
the `TrackEventConfig`, so the SDK's heaviest startup tracing user never goes
near this path.
The strip only helps SDKs new enough to have it. On an older SDK the alias
arrives as an unknown field, and the generated `operator==` compares
`unknown_fields_` along with the known ones, so the comparison fails there, and
the service cannot do anything about it from its side.
### ChangeTraceConfig
Logging this one too. `ChangeTraceConfig()` lets a consumer update the producer
filters on a running session, and it finds the entry to update by type name,
first match wins:
```cpp
// src/tracing/service/tracing_service_impl.cc
for (const auto& it : updated_cfg.data_sources()) {
if (cfg_data_source.config().name() == it.config().name()) {
new_producer_name_filter = it.producer_name_filter();
...
break;
}
}
```
Against the config in the Problem section, that is already wrong. Both
`linux.process_stats` entries come out with the first updated entry's filter,
and whatever the second one said is dropped without a word. So this is not an
ambiguity that RFC-0028 introduces; it is in the service today.
## Alternatives considered
### Other names for the field
`name` is taken and cannot be reused, so this needs a name of its own. The
candidates:
- `alias`, proposed. Short, reads correctly in "refer to the data source by its
alias", and familiar to anybody who has written SQL `AS`.
- `instance_name`. Misleading. One `data_sources {}` entry can turn into several
runtime `DataSourceInstance`s, one per producer matched by
`producer_name_filter`. This is a config-level handle, not the name of any
single instance.
- `id`. Implies numeric, or globally unique, or both. This is a string, scoped
to one config.
- `label`. Sounds like display metadata, the sort of thing a UI renders. This is
something that references resolve against.
The fair objection to `alias` is that an alias is normally an alternative name
for something that already has one, and here the entry has no name of its own.
That is true. I still think it is the best of the four: it matches how `alias`
gets used in configuration languages generally, and none of the others are
better on balance.
### Generating an alias when unset
The service could make up a handle for entries that don't set one, something
along the lines of `linux.process_stats#2`.
Rejected. An implicit handle that changes when entries get reordered has exactly
the fragility of the positional index we are trying to get away from, except now
it also looks stable.
Hashing the entry instead of counting it survives reordering, but an alias gets
typed by one person and read by another. `linux.process_stats@3f2a91` buys
nothing over the index, and it changes whenever somebody edits the entry.
### Global uniqueness
Making aliases unique across sessions, rather than within one, means a config
that worked yesterday can fail today purely because some unrelated session
happened to pick the same alias first. Consumers don't know about each other, so
the only way to avoid those collisions would be a naming convention shared by
every consumer on the device, and no such thing exists.
Per-session scoping also keeps validation local to `EnableTracing`, which is
where the rest of the config validation already lives.
## Open questions
- Is `alias` the right name? I fully expect this to be the part that gets
argued about, and that is most of the reason this is written down before the
first consumer lands rather than after.
- Should the alias reach the producer at all? Putting it on `DataSourceConfig`
rather than on the wrapper is what carries it into the producer's process, and
that is also the only reason it costs anything on the C++ SDK.
Two ways out: put it on the wrapper, or leave it where it is and have
`TracingServiceImpl::SetupDataSource()` clear it before sending the config on,
right where the service already stamps `target_buffer` and
`tracing_session_id`. That stamping code has a comment about the same
failure mode, on a field the service sets itself:
```cpp
// src/tracing/service/tracing_service_impl.cc
// Rationale for `if (prefer) set_prefer(true)`, rather than `set(prefer)`:
// [...] Unconditionally adding a new field breaks backward compatibility of
// startup tracing with older SDKs, because the serialization also
// propagates unkonwn fields, breaking the hash matching check.
if (tracing_session->config.prefer_suspend_clock_for_duration())
ds_config.set_prefer_suspend_clock_for_duration(true);
```
Either way, startup adoption keeps working on old and new SDKs, and the strip
list change goes away. The trade is that the producer never sees the alias,
so an instance cannot log which config entry it came from. That is the only
use I have for it there so far anyway.