Skip to content

[Usage Collection] [schema] apm#79000

Merged
afharo merged 8 commits intoelastic:masterfrom
afharo:usage_collection/schema/apm
Oct 2, 2020
Merged

[Usage Collection] [schema] apm#79000
afharo merged 8 commits intoelastic:masterfrom
afharo:usage_collection/schema/apm

Conversation

@afharo
Copy link
Copy Markdown
Member

@afharo afharo commented Sep 30, 2020

Summary

  • Add schema definition to the collector apm.
  • @kbn/telemetry-tool: Add support to Record<'prop1' | 'prop2', SOMETHING> (previously only supporting Record<string, SOMETHING>).
  • @kbn/telemetry-tool: Add support to Pick<SOMETHING, 'prop1' | 'prop2'>.

Related to #70180.

Closes #72736.

Checklist

For maintainers

@afharo afharo added Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:KibanaTelemetry v7.10.0 v8.0.0 labels Sep 30, 2020
APM_TELEMETRY_SAVED_OBJECT_ID,
APM_TELEMETRY_SAVED_OBJECT_TYPE,
} from '../../../common/apm_saved_object_constants';
import { getApmTelemetryMapping } from '../../../common/apm_telemetry';
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.

Maybe we can delete that method, and stop maintaining the script upload-telemetry-data. It can be deleted on a follow up PR, though. What do you think @elastic/apm-ui?

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.

We use upload-telemetry-data to test if changes we make in development actually work. Couldn't we keep it have it use apmSchema? Not sure why we would want to delete it unless we have a better way to verify that are tasks work in development.

Copy link
Copy Markdown
Member Author

@afharo afharo Oct 1, 2020

Choose a reason for hiding this comment

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

@smith sure! If the APM team uses upload-telemetry-data for additional purposes other than provide an up-to-date mappings to the telemetry cluster. I have nothing against keeping it. That's also why I didn't remove it in this PR. It's your call 🙂

schema provides a double validation: TS and Telemetry-Tools about the data matching the structure. I understand if you also want to use the upload-telemetry-data for functional tests.

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 upload-telemetry-data is not used to provide mappings to the telemetry cluster; it's only used in development.

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.

Got it! I agree then! There's no need to remove it. Sorry for the misunderstanding 😇

@afharo afharo marked this pull request as ready for review September 30, 2020 19:04
@afharo afharo requested a review from a team September 30, 2020 19:04
@afharo afharo requested a review from a team as a code owner September 30, 2020 19:04
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/apm-ui (Team:apm)

@afharo afharo added the review label Sep 30, 2020
Copy link
Copy Markdown
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

This looks great! Could you please remove the getApmTelemetryMapping() function and its tests and update mergeApmTelemetryMapping to use the APM scheme here? https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/common/apm_telemetry.ts#L271

@afharo afharo requested a review from smith October 2, 2020 12:30
@afharo
Copy link
Copy Markdown
Member Author

afharo commented Oct 2, 2020

@smith as requested, I've updated getApmTelemetryMapping() to build the mappings now out of the schema

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

distributable file count

id before after diff
default 45824 45825 +1

History

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

@afharo afharo merged commit 9973667 into elastic:master Oct 2, 2020
@afharo afharo deleted the usage_collection/schema/apm branch October 2, 2020 18:24
afharo added a commit that referenced this pull request Oct 2, 2020
smith added a commit to smith/kibana that referenced this pull request Oct 5, 2020
In elastic#79000 the way we create the telemetry schema was updated and APM is no longer excluded from the telemetry scripts.

Update the docs to be more correct about this.
@smith smith mentioned this pull request Oct 5, 2020
smith added a commit that referenced this pull request Oct 6, 2020
In #79000 the way we create the telemetry schema was updated and APM is no longer excluded from the telemetry scripts.

Update the docs to be more correct about this.
smith added a commit to smith/kibana that referenced this pull request Oct 6, 2020
In elastic#79000 the way we create the telemetry schema was updated and APM is no longer excluded from the telemetry scripts.

Update the docs to be more correct about this.
smith added a commit that referenced this pull request Oct 12, 2020
In #79000 the way we create the telemetry schema was updated and APM is no longer excluded from the telemetry scripts.

Update the docs to be more correct about this.
@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 review Team:APM - DEPRECATED Use Team:obs-ux-infra_services. 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.

Add schema to APM data telemetry

6 participants