Somehow I forgot to document this when I added this option 🙈 Added it now!

We need to register this type to include its mappings in the .kibana index, but we set up this service before the SavedObjectsService. So I exposed a function here to allow the SavedObjectsService to register this type and retain separation of concerns.

I thought the attributes for this saved object type should be structured in a flexible manner, so in the future we can:

1. Record telemetry data for other API calls
2. Record telemetry data for something besides API calls

Each of these telemetry client functions has three steps:

1. Retrieve current telemetry data (or an empty attributes object, if no current telemetry data exists)
2. Update the attributes, incrementing whatever counter field(s) are necessary
3. Persist telemetry data to Elasticsearch

There is a race condition if two users call the same API at the same time (either in a single Kibana server, or separate Kibana servers in a HA configuration). However, the impact is minimal, in that edge case we'll simply lose one of the increments. I think that's an acceptable risk.

The alternative is using the document version to ensure the saved object hasn't changed before we update it, and in that scenario, calculate new attributes so we can retry. Seems like overkill to me, though.

Since CoreUsageData extends CoreTelemetry now, we have to export CoreTelemetry here. However, the CoreTelemetryService is only intended to be used by Core code, so we don't have to export anything else at the src/core/server level.

Edit: see #83575 (comment), I renamed CoreTelemetry to CoreUsageStats.

Here is the setup/start process:

1. CoreTelemetryService CoreUsageStatsService setup (depends on SavedObjectsService start)
2. SavedObjectsService setup (depends on CoreTelemetryService CoreUsageStatsService setup)
3. CoreUsageDataService setup (depends on CoreTelemetryService CoreUsageStatsService setup)
4. SavedObjectsService start
5. CoreUsageDataService start, immediately fetching usage data and querying for any persisted telemetry data usage stats in the process

The CoreTelemetryService CoreUsageStatsService needs a SavedObjectsRepository so it can persist telemetry data to Elasticsearch. I spoke with @pgayvallet and we determined the best way to handle this messy circular dependency is to pass a promise to the CoreTelemetryService CoreUsageStatsService which resolves after the SavedObjectsService is started.

Edit: changed type names based on PR review feedback.

nit: savedObjectsStartPromiseResolver is just a function, rather than an instance of a class, so I would make that clearer with the naming. e.g. resolveSavedObjectsStartPromise

Done in 7ecb815, thanks!

This fixes a bug that I didn't run into until I changed createNewCopies to be enabled by default on the client side.

nit: savedObjectsStartPromiseResolver is just a function, rather than an instance of a class, so I would make that clearer with the naming. e.g. resolveSavedObjectsStartPromise

I feel like this class is doing the exact same thing as the CoreTelemetryClient except that it's tailored to the Spaces use case.

Maybe instead of having many of these clients, each one implementing basically the same logic, we could just have one telemetry service in core that every plugin can use to add telemetry data.

As far as I can tell the logic is the following when we want to record usage data:

1. Register saved object type mappings for each use case / plugin
2. Retrieve saved object
3. Increment count (a bit like a redux reducer)
4. Update saved object

Something like this could encapsulate this logic:

// Plugin setup
const update = core.telemetryService.register(name, mappings);

// Route handler
await update((currentAttributes = defaultAttributes) => nextAttributes);

e.g. for Spaces plugin:

// Plugin setup
const updateSpacesUsage = core.telemetryService.register('spaces-telemetry', mappings);

// Route handler
await updateSpacesUsage((current = CREATE_NEW_COPIES_DEFAULT) => ({
  total: current.total + 1,
  createNewCopies: incrementBooleanCount(current.createNewCopies, createNewCopies),
  overwrite: incrementBooleanCount(current.overwrite, overwrite),
}))

Just thinking out loud here, but this would make it a lot easier to add additional telemetry events. The only thing you'd need to implement and test as a plugin developer would be the reducer function.

I like the idea and it sounds elegant, and I agree this is essentially duplicated code. However, I know we are trying hard to keep the core API surface as small as possible, and I'm not sure we want to add a whole service for something that we don't yet have additional use cases for. I'll defer to @elastic/kibana-core since they would be the owners of said service.

getUsageStats() silently swallows any exceptions and returns an empty object. So if there's an exception in getUsageStats we will reset all counters to 0. If we can't get the existing stats it would be better not to do an update operation (thereby losing one increment instead of all previous counters).

This behaviour is present in the other methods too (and the Core service), just leaving one comment for all of them.

Good point, sounds like this will be resolved when using incrementCounter per #83575 (comment)

Question: is this yes/no format required (or here for consistency)? As total = yes + no, I feel like one of these fields may be redundant?