Add usage collection for savedObject tagging #83160

pgayvallet · 2020-11-11T12:35:43Z

Summary

Add a new saved_objects_tagging usage collector to monitor tagging usages.

Data structure:

{
  usedTags: number;
  taggedObjects: number;
  types: Record<string, { usedTags: number; taggedObjects: number;}> 
}

Sample data:

{
  usedTags: 3;
  taggedObjects: 7;
  types: {
    dashboard: {
       usedTags: 3;
       taggedObjects: 5;
    },
    visualization: {
       usedTags: 1;
       taggedObjects: 2;
    },
  }
}

Checklist

Unit or functional tests were updated or added to match the most common scenarios

x-pack/plugins/saved_objects_tagging/server/usage/tag_usage_collector.ts

x-pack/plugins/saved_objects_tagging/server/usage/fetch_tag_usage_data.ts

elasticmachine · 2020-11-11T18:32:07Z

Pinging @elastic/kibana-platform (Team:Platform)

…ge-collector

x-pack/plugins/saved_objects_tagging/server/usage/tag_usage_collector.ts

Bamieh

LGTM. Mergable after removing the esClient check or we can remove it once we fix the bug.

…ge-collector

TinaHeiligers

LGTM now that the esClient check is removed.

afharo · 2020-11-18T18:03:30Z

FYI: the missing esClient issue will be gone once #83546 is merged :)

rudolf · 2020-11-19T11:42:48Z

x-pack/plugins/saved_objects_tagging/server/usage/schema.ts

+  taggedObjects: { type: 'integer' },
+
+  types: {
+    dashboard: perTypeSchema,


we collect tags usage data for more than just these two saved object types. I think we should try to avoid having this kind of dynamic fields in our schema and it will also increase the field count on our telemetry cluster.

What about:

perSavedObjectType: { usedTags: { type: 'integer' }, taggedObjects: { type: 'integer' }, savedObjectType: {type: 'string'}, }

perSavedObjectType: { ...

Should we use a nested or array type in that case?

That would work. However I would be sure that is would be alright to the way telemetry consumes / uses the data.

When I look at the other collectors, none seems to be used nested fields, which is why I'm asking:

kibana/src/plugins/kibana_usage_collection/server/collectors/application_usage/schema.ts

Lines 52 to 59 in d67a421

export const applicationUsageSchema = {

// OSS

dashboards: commonSchema,

dev_tools: commonSchema,

discover: commonSchema,

home: commonSchema,

kibana: commonSchema, // It's a forward app so we'll likely never report it

management: commonSchema,

note that this could only be a design flaw when the collector was implemented. @elastic/kibana-telemetry?

Hey! This is a legacy issue that no longer applies: in the past, before having the schema implementation, the indexer in the remote service indexed the data in the same format it received it from Kibana. This meant that we had to keep the mappings in mind when developing the collectors (and nested was never an option because Kibana does not natively support viz with that type yet).
New collectors/fields shouldn't have those contraints anymore because the new indexer actually reshapes the payloads and splits the info into multiple indices based on the different needs to consume the information by our PMs and Analysts.
If we foresee the number of keys growing dynamically, I think the array approach would be best.

cc @mindbat so he can share his thoughts from the indexer-maintainer POV 🙂

New collectors/fields shouldn't have those contraints anymore because the new indexer actually reshapes the payloads and splits the info into multiple indices based on the different needs to consume the information by our PMs and Analysts.

@afharo: We're not sending Kibana data to Telemetry Next yet and the legacy indexer doesn't reshape data by default. We would still need to update the indexer to consume the new data, reshape it and index it into a custom extended index.

So should we stick with the static, per type, mappings for now?

Even if we just index data as-is, can you elaborate why we couldn't use an array field type for the mappings?

and nested was never an option because Kibana does not natively support viz with that type yet

I missed @afharo's comment and I assume this applies to array fields too #58175

It makes sense that we want to do some processing inside Telemetry Next, but I think it would be much more flexible if Kibana had more control around which documents are created. If telemetry is uploaded as NDJSON then usage collectors could return several documents instead of always having a single large payload that needs to be handled and split by the server. This is probably a big effort, but maintaining a static schema for dynamic data like applications and saved object types that change every release feels like a maintenance nightmare.

So I guess for this PR we just need to add all the known saved object types to the mappings.

So I guess for this PR we just need to add all the known saved object types to the mappings.

We only need to add types that currently supports the tagging feature. That would just be dashboard, visualizations and maps. Will add maps. Further additions should only be done when the app/type implements the tagging feature.

Done in 7b6d76e: added map to the schema, and updated the readme to state that developers should update the schema when adding tagging support to additional SO types.

@rudolf

Even if we just index data as-is, can you elaborate why we couldn't use an array field type for the mappings?

Without any transformations on the remote service indexer, we'd end up with data mapped as an array in elasticsearch and face the usual problem of not being able to meaningfully query or visualize that data. It's a situation we're trying to fix with ui_metric right now, for example.

rudolf · 2020-11-19T11:45:34Z

x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json

@@ -1841,6 +1817,30 @@
        }
      }
    },
+    "infraops": {


@elastic/kibana-telemetry Out of curiosity, any idea why the order of fields are changing here? Would be nice if we could make it stable to avoid unnecessary diffs.

@Bamieh do you have any idea how we can prevent reordering fields in the json files?

…ge-collector

kibanamachine · 2020-11-20T15:01:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7b6d76e

Metrics [docs]

Distributable file count

id	before	after	diff
`default`	42957	42962	+5

History

💚 Build #88269 succeeded 4042358
💚 Build #87902 succeeded 70d58d3
💚 Build #87546 succeeded af1135f
💚 Build #87355 succeeded 59716f8
💔 Build #87324 failed f62ea18

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

* add so tagging usage collection * update telemetry mappings * fix types * remove check on esClient presence * update schema and README

* master: (38 commits) [ML] Data frame analytics: Adds functionality to map view (elastic#83710) Add usage collection for savedObject tagging (elastic#83160) [SECURITY_SOLUTION] 145: Advanced Policy Tests (elastic#82898) [APM] Service overview transactions table (elastic#83429) [ML] Fix Single Metric Viewer not loading if job is metric with no partition (elastic#83880) do not export types from 3rd party modules as 'type' (elastic#83803) [Fleet] Allow to send SETTINGS action (elastic#83707) Fixes Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details·ts - Actions and Triggers app Alert Details Alert Instances renders the active alert instances (elastic#83478) [Uptime]Reduce chart height on monitor detail page (elastic#83777) [APM] Prefer `APIReturnType` over `PromiseReturnType` (elastic#83843) [Observability] Fix telemetry for Observability Overview (elastic#83847) [Alerting] Adds generic UI for the definition of conditions for Action Groups (elastic#83278) ensure workload agg doesnt run until next interval when it fails (elastic#83632) [ILM] Policy form should not throw away data (elastic#83077) [Monitoring] Stop collecting Kibana Usage in bulkUploader (elastic#83546) [TSVB] fix wrong imports (elastic#83798) [APM] Correlations UI POC (elastic#82256) list all the refs in tsconfig.json (elastic#83678) Bump jest (and related packages) to v26.6.3 (elastic#83724) Functional tests - stabilize reporting tests for cloud execution (elastic#83787) ...

* Add usage collection for savedObject tagging (#83160) * add so tagging usage collection * update telemetry mappings * fix types * remove check on esClient presence * update schema and README * fix mappings for backport

pgayvallet added 3 commits November 11, 2020 13:33

add so tagging usage collection

f62ea18

update telemetry mappings

a6ae4a9

fix types

59716f8

pgayvallet commented Nov 11, 2020

View reviewed changes

x-pack/plugins/saved_objects_tagging/server/usage/tag_usage_collector.ts Outdated Show resolved Hide resolved

x-pack/plugins/saved_objects_tagging/server/usage/fetch_tag_usage_data.ts Show resolved Hide resolved

pgayvallet marked this pull request as ready for review November 11, 2020 18:31

pgayvallet added Feature:Saved Object Tagging Saved Objects Tagging feature release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.11.0 v8.0.0 labels Nov 11, 2020

pgayvallet requested review from a team November 11, 2020 18:32

Merge remote-tracking branch 'upstream/master' into kbn-81847-tag-usa…

af1135f

…ge-collector

pgayvallet mentioned this pull request Nov 12, 2020

Usage Analytics on Saved Object Tags #81847

Closed

Merge remote-tracking branch 'upstream/master' into kbn-81847-tag-usa…

70d58d3

…ge-collector

Bamieh reviewed Nov 16, 2020

View reviewed changes

x-pack/plugins/saved_objects_tagging/server/usage/tag_usage_collector.ts Outdated Show resolved Hide resolved

Bamieh approved these changes Nov 16, 2020

View reviewed changes

pgayvallet added 2 commits November 17, 2020 12:25

Merge remote-tracking branch 'upstream/master' into kbn-81847-tag-usa…

93ae79d

…ge-collector

remove check on esClient presence

4042358

rudolf mentioned this pull request Nov 17, 2020

Reasons for not using saved objects for storing kibana data #80912

Open

TinaHeiligers approved these changes Nov 17, 2020

View reviewed changes

pgayvallet requested a review from rudolf November 18, 2020 06:59

rudolf requested changes Nov 19, 2020

View reviewed changes

pgayvallet added 2 commits November 20, 2020 13:50

Merge remote-tracking branch 'upstream/master' into kbn-81847-tag-usa…

0235025

…ge-collector

update schema and README

7b6d76e

rudolf approved these changes Nov 20, 2020

View reviewed changes

pgayvallet merged commit 22e494e into elastic:master Nov 20, 2020

pgayvallet mentioned this pull request Nov 20, 2020

[7.x] Add usage collection for savedObject tagging (#83160) #83924

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add usage collection for savedObject tagging #83160

Add usage collection for savedObject tagging #83160

pgayvallet commented Nov 11, 2020 •

edited

Loading

elasticmachine commented Nov 11, 2020

Bamieh left a comment

TinaHeiligers left a comment

afharo commented Nov 18, 2020

rudolf Nov 19, 2020

pgayvallet Nov 19, 2020

afharo Nov 19, 2020

TinaHeiligers Nov 19, 2020

pgayvallet Nov 19, 2020

rudolf Nov 19, 2020 •

edited

Loading

rudolf Nov 20, 2020

pgayvallet Nov 20, 2020

pgayvallet Nov 20, 2020

TinaHeiligers Nov 20, 2020

rudolf Nov 19, 2020

TinaHeiligers Nov 19, 2020

kibanamachine commented Nov 20, 2020

	export const applicationUsageSchema = {
	// OSS
	dashboards: commonSchema,
	dev_tools: commonSchema,
	discover: commonSchema,
	home: commonSchema,
	kibana: commonSchema, // It's a forward app so we'll likely never report it
	management: commonSchema,

Add usage collection for savedObject tagging #83160

Add usage collection for savedObject tagging #83160

Conversation

pgayvallet commented Nov 11, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Nov 11, 2020

Bamieh left a comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

afharo commented Nov 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudolf Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Nov 20, 2020

💚 Build Succeeded

Metrics [docs]

Distributable file count

History

pgayvallet commented Nov 11, 2020 •

edited

Loading

rudolf Nov 19, 2020 •

edited

Loading