Skip to content

[Telemetry] Add Application Usage Schema#75283

Merged
afharo merged 9 commits intoelastic:masterfrom
afharo:telemetry/application-usage_schema
Aug 26, 2020
Merged

[Telemetry] Add Application Usage Schema#75283
afharo merged 9 commits intoelastic:masterfrom
afharo:telemetry/application-usage_schema

Conversation

@afharo
Copy link
Copy Markdown
Member

@afharo afharo commented Aug 18, 2020

Summary

Adds the usageCollection.schema option to the Application Usage collector.

In order to do so, a few additional changes had to be made in the @kbn/telemetry-tools package in order to support IndexSignatures in interfaces.

Resolves #70622

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@afharo afharo marked this pull request as ready for review August 18, 2020 12:54
@afharo afharo requested a review from a team as a code owner August 18, 2020 12:54
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-telemetry (Team:KibanaTelemetry)

// TODO: Find a way to update these keys automatically.
export const applicationUsageSchema = {
// OSS
dashboards: commonSchema,
Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers Aug 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a few application ids missing:
dashboard (we have both singular and plural)
security_capture_url
siem
alerting_fixture

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TinaHeiligers, thank you for the ID check.
Q: Did you find them in the Telemetry data from previous versions or in the code?

In the past, we had both: dashboard and dashboards but from 7.9 we only have the plural version of it. siem has been replaced with all the securitySolution:* keys. I tried to keep in here the schema for the active version only.

security_capture_url: I didn't include it because it's a chromeless app with a redirection, so we'll likely don't report anything. Maybe it's worth adding them anyway 🤔

alerting_fixture: is a plugin defined in the x-pack/test directory. Do we need to report those as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afharo

Q: Did you find them in the Telemetry data from previous versions or in the code?

In the code, running the master branch.

The others we can leave out.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added all of them (including the forwarding apps) for documentation. We can come back to exclude them later on if we want to 🙂

@TinaHeiligers thank you for noticing 👀

Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a first pass, I found a few applications missing from the schema declaration. Code-wise, I'm still reviewing.

@afharo
Copy link
Copy Markdown
Member Author

afharo commented Aug 21, 2020

@elasticmachine merge upstream

@afharo
Copy link
Copy Markdown
Member Author

afharo commented Aug 24, 2020

@elasticmachine merge upstream

@afharo afharo requested a review from a team August 24, 2020 13:48
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
@Bamieh is working on the schema AST, so he might want to take a look too.

Copy link
Copy Markdown
Contributor

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. i love this improvement

@afharo afharo merged commit 86d7050 into elastic:master Aug 26, 2020
@afharo afharo deleted the telemetry/application-usage_schema branch August 26, 2020 11:51
afharo added a commit that referenced this pull request Aug 27, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@lukeelmers lukeelmers added the Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// label Oct 1, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Telemetry] [Collectors] Application Usage: Define collector.schema

6 participants