[data views] add usage counters to data views REST api endpoints

[mattkime]

Summary

Add usage counters to data views (and legacy index patterns) REST endpoints.

Adding usageCounter calls to the code was trivial but testing it turned out to be very difficult. Functional testing usageCounters turned out to be idiosyncratic and painful so I switched to unit tests which required a fair amount of code refactoring. The jest tests are only checking for usageCounter calls but the functional / api integration tests cover everything else.

Closes: #100185

[elasticmachine]

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

[mattkime]

@elasticmachine merge upstream

[TinaHeiligers]

While counterType and incrementBy are optional, you could add even more detail by using counterType as error or legacyError or something along those lines.

There are a few plugins that use counterType to describe the stats, e.g. alerting uses 'legacyTerminology' to track legacy terms.
Here we could do something similar:

usageCollection?.incrementCounter({
        counterName: path,
        counterType: 'legacyError'
      });

[TinaHeiligers]

The changes look fine to me but I'd be more confident if we had tests for the implementation. I'll leave that decision up to your team though.

[jloleysens]

Did not test this locally, but the code LGTM. Left minor nits and a question about implementation that might be interesting to consider for future.

[jloleysens]

nit; this existed before, but it is not necessary to call JSON.stringify here.

[jloleysens]

nice!

[jloleysens]

@mattkime what do you think of accessing the path pattern at the http service level? Dreaming here:

    http.registerOnPreResponse((req, res, t) => {
      //                                                                      t.matchedRoute does not exist
      usageCollection?.incrementCounter({ counterName: `${req.route.method} ${t.matchedRoute.path}` });
      return t.next();
    });

I'm not sure how common this need is (or what other things that might address). If you agree though perhaps we can suggest it to core. I may also be misunderstanding when onPreResponse is occurring.

[mattkime]

@jloleysens thats an interesting idea - I'll talk to core about it

[mattkime]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: dcd8cbc
• Storybooks Preview
• Webpack Bundle Analyzer

Metrics [docs]

✅ unchanged

History

• 💚 Build #20183 succeeded 51f0f66
• 💚 Build #20120 succeeded 03378cb
• 💔 Build #19965 failed 3648ebd
• 💔 Build #19962 failed 5305e7d
• 💚 Build #19950 succeeded 3800d9f
• 💛 Build #19755 was flaky edf0018

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

[jloleysens]

Great work @mattkime ! I'm approving to unblock progress but a couple ideas came to mind as I was re-reviewing this:

It looks like this implementation will not capture 400 responses that do not make it past the schema check - I'm guessing this would be a small-ish number overall so should be OK! But it is usage info that we can't track.

The other idea was: it still feels like we could address this at a slightly higher level of abstraction. The weakness there would be dealing with exceptions like only wanting to capture this info for a subset of routes, but it might still be a bit less work. Something like an "enhanced" router where we wrapped get, put, post and wrapped the route handler in a function that does the incrementCounter call. But perhaps that does not fit with what you are trying to capture. Just wanted to share the idea :) - I also see the advantage of the logic at a lower level - easier to adapt if needed per case.

[EDIT]

Also did a smoke-test and everything looks to be working as expected!

[Dosant]

Code lgtm,

It will be very useful to know if API is called by Kibana or by someone else.
This way we will know the usage outside of Kibana and can make better decisions about public APIs.

Do we have this reported?
If not automatically, maybe we can add a flag or something based on headers?

[Dosant]

seems like this isn't used

[mattkime]

@jloleysens

The other idea was: it still feels like we could address this at a slightly higher level of abstraction

Agreed, just didn't make it in but I'll create a follow up. It seems like this use case (more end points, tracking) will become more common.

It will be very useful to know if API is called by Kibana or by someone else.

As best I know there aren't any internal consumers but its a good idea that I'll put into a follow up.