[Telemetry] Application Usage implemented in @kbn/analytics

[afharo]

Summary

Add a plugin that retrieves the time (in minutes) and the number of clicks on each application inside Kibana and report it via telemetry.

It uses its own index to store the grouped events in the form of { timestamp, appId, numberOfClicks, minutesOfScreenTime }. Rolling up the information to saved objects for the entries older than 90 days (because the required reported data has 3 levels of aggregation: 30 days, 90 days and total).

The finally reported information to our telemetry service looks like below:

    "stack_stats": {
      "kibana": {
        "plugins": {
           ...
           "application_usage": {
             "logs": {
                 "clicks_total": 1000,
                 "clicks_30_days": 10,
                 "clicks_90_days": 100,
                 "minutes_on_screen_total": 58.93,
                 "minutes_on_screen_30_days": 10.23,
                 "minutes_on_screen_90_days": 30.0,
             },
             "apm": { ...},
             ...
           }
           ...
       }
  }

Fixes #52737

Checklist

Delete any items that are not applicable to this PR.

• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for cross-browser compatibility, including a check against IE11

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[elasticmachine]

Pinging @elastic/pulse (Team:Pulse)

[afharo]

I'm pending to write tests to this alternative implementation.

@Bamieh the bit I like #57893 best is that the implementation is self-contained in one plugin, whereas in this one the logic is spread across 3 entities:

• @kbn/analytics for the frontend collection logic: listeners + localStorage + batch reporting to the API
• usage_collector for the API handling (storing into Saved Objects)
• telemetry for the usage collection logic (aggregate documents from the Saved Objects + roll up indices old documents.

But I get it that this one reuses many of the capabilities already implemented and tested in those 3 components 👍

[afharo]

@elasticmachine merge upstream

[Bamieh]

LGTM, lets get it merged!

Just a thought: I wish we had the event listeners in the application_usage/public plugin instead of kbn-analytics/src/reporter.ts and then these listeners would call the kbn analytics methods to update usage state.

Thanks for all the thorough tests!

[afharo]

I wish we had the event listeners in the application_usage/public plugin instead of kbn-analytics/src/reporter.ts

I thought about it but then I thought it was maybe not a good idea to publicly expose the "add clicks" or like the common logic when it's supposed to send the report (similar to the setInterval for sending the report sits in the reporter instead of the plugin).

But I don't have a strong opinion about it. Happy to revisit this at any point.

Similarly, we should start adding tests to that side library at some point 😅(especially if it's going to grow over time). But we can do it on a separate PR if that's fine :)

[TinaHeiligers]

Aside and not for this PR: We might want to consider adding a generic type that devs can use for metrics not falling into any of the current metric types. For example: the recent File Upload migration to the NP includes a a request to add File Upload as a type. We should think about how to handle cases like these where we might not necessarily allow an overwrite of the generic type but rather have that if a generic type is used, there must be another descriptor (like a description) tag in the data we're capturing. e.g. metric type='generic', and description='File Upload`.

[afharo]

That's a fair comment. I think the main reason why we haven't used the UIStatsMetrics more is because of the way we report them (as arrays).

I'm sure we'll be coming back to this at some point with the new Pulse approach where arrays won't be an issue anymore.

[TinaHeiligers]

In an ideal world, we'd be adding the new collector directly in the NP and not extending anything in the Legacy world.
One nit to add yet more tests and I know I'm being greedy! Feel free to add later.
LGTM!

[TinaHeiligers]

nit: We should also be testing internal methods used when we are returning the 'something'. i.e: rollTotals.

[TinaHeiligers]

We're using this method to aggregate data and it should at least have a rudimentary test since it's also using findAll.

[afharo]

In an ideal world, we'd be adding the new collector directly in the NP and not extending anything in the Legacy world.

You are right! I feel bad for having all these new additions to the Legacy folder. Good thing it's using the NP components internally, so migrating them should be a piece of cake once we solve all the other challenges (i.e.: the registered routes at the moment rely heavily on the Legacy code). The public side of the telemetry plugin has already been migrated to NP. It's only the server side the one that is missing at this point. Maybe we should bring back up in the priorities #38765

Since I have a fresh memory regarding this piece of code. I can volunteer to do the changes.

One nit to add yet more tests and I know I'm being greedy! Feel free to add later.

You are totally right! I was force-running it via jest.runTimersToTime(ROLL_INDICES_INTERVAL);, but not testing the outcoming results (and actually found a bug that I was adding the totals on every iteration so we were coming up with a number way higher than expected). Just added a check at the mock to ensure we are storing the right data.
You should be able to see the changes in this commit

LGTM!

Thank you for your review :)

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: c53a740

History

• 💚 Build #29680 succeeded d7e8313
• 💚 Build #29204 succeeded 56906b0
• 💔 Build #29086 failed ad91cfe
• 💚 Build #28810 succeeded 528cb7f
• 💔 Build #28742 failed a4d73cd

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)