Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[telemetry] add schema guideline + schema_check new check for --path config #75747

Merged
merged 5 commits into from
Sep 14, 2020
Merged

[telemetry] add schema guideline + schema_check new check for --path config #75747

merged 5 commits into from
Sep 14, 2020

Conversation

Bamieh
Copy link
Member

@Bamieh Bamieh commented Aug 24, 2020

Add a schema guideline to help teams update their collector schemas.

Context: #70180

@Bamieh Bamieh added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:KibanaTelemetry v7.10.0 labels Aug 24, 2020
@Bamieh Bamieh requested a review from a team as a code owner August 24, 2020 12:50
@elasticmachine
Copy link
Contributor

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

afharo
afharo reviewed Aug 24, 2020
View reviewed changes
Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Great piece of work! I'd just like to better understand some comments and questions I've got before giving the final LGTM 🙂

Also added some NITs that I think we can improve in separate issues.

packages/kbn-telemetry-tools/GUIDELINE.md Show resolved Hide resolved
packages/kbn-telemetry-tools/GUIDELINE.md Outdated Show resolved Hide resolved
packages/kbn-telemetry-tools/GUIDELINE.md Show resolved Hide resolved
packages/kbn-telemetry-tools/src/cli/run_telemetry_check.ts Outdated Show resolved Hide resolved
packages/kbn-telemetry-tools/src/cli/run_telemetry_check.ts
Comment on lines +70 to +85
{
enabled: () => typeof path !== 'undefined',
title: 'Checking collectors in --path are not excluded',
task: ({ roots }: TaskContext) => {
const totalCollections = roots.reduce((acc, root) => {
return acc + (root.parsedCollections?.length || 0);
}, 0);
const collectorsInPath = Array.isArray(path) ? path.length : 1;

if (totalCollections !== collectorsInPath) {
throw new Error(
'Collector specified in `path` is excluded; Check the telemetryrc.json files.'
);
}
},
},
Copy link
Member

Choose a reason for hiding this comment

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

Just to fully understand this piece of code: Each path should only contain one collector for this to work. i.e.: I can't run --path=src/plugins/kibana_usage_collection

Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. It is documented in the README as well:

  • --path specifies a collector path instead of checking all collectors specified in the .telemetryrc.json files. Accepts a .ts file. The file must be discoverable by at least one rc file.

src/plugins/usage_collection/server/collector/collector.ts Outdated Show resolved Hide resolved
@afharo afharo mentioned this pull request Aug 24, 2020
Merged
@Bamieh Bamieh requested a review from afharo September 14, 2020 11:51
@Bamieh Bamieh changed the title [telemetry] add schema guideline [telemetry] add schema guideline + schema_check new check for --path config Sep 14, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

distributable file count

id value diff baseline
default 45519 +1 45518
oss 27224 +1 27223
total +2

History

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

afharo
afharo approved these changes Sep 14, 2020
View reviewed changes
Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

LGMT!

@Bamieh Bamieh merged commit 8eb4b2e into elastic:master Sep 14, 2020
@Bamieh Bamieh deleted the telemetry/schema_guideline branch September 14, 2020 14:39
Bamieh added a commit to Bamieh/kibana that referenced this pull request Sep 14, 2020
@Bamieh
[telemetry] add schema guideline + schema_check new check for --path …
24fb382 
…config (elastic#75747)
@Bamieh Bamieh mentioned this pull request Sep 14, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
@gmmorris
Merge branch 'master' into alerting-exempt-rbac
a0fa54e 
* master: (26 commits)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  [ML] Functional tests - increase wait time for DFA start (elastic#77307)
  [UiActions][Drilldowns] Fix actions sorting in context menu (elastic#77162)
  [Drilldowns] Wire up new links to new docs (elastic#77154)
  Fix APM issue template
  [Ingest Pipelines] Drop into an empty tree (elastic#76885)
  ...
Bamieh added a commit that referenced this pull request Sep 14, 2020
@Bamieh
[7.x] [telemetry] add schema guideline + schema_check new check for -…
0764fc7 
…-path config (#75747) (#77358)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
@gmmorris
Merge branch 'master' into actions/test-connector
91ea9c5 
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
@gmmorris
Merge branch 'master' into alerts/improve-find-pref
b4729a4 
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
@lukeelmers lukeelmers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Oct 1, 2021
@elasticmachine
Copy link
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
Reviewers

@afharo afharo afharo approved these changes

Assignees
No one assigned
Labels
Feature:Telemetry 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.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Bamieh @elasticmachine @kibanamachine @afharo @lukeelmers