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.

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

[Reporting/telemetry] Add 'statuses' object to usage to show status counts by jobType & appType #63922

Merged

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Apr 18, 2020

Summary

This PR follows a request from @elastic/kibana-canvas to have Reporting usage data show the status type per app integration.

Example:

    "statuses": Object {
      "completed": Object {
        "PNG": Object {
          "dashboard": 1,
        },
        "csv": Object {},
        "printable_pdf": Object {
          "canvas workpad": 1,
          "dashboard": 1,
        },
      },
    },

NOTE: CSV numbers are not available for the type of search aggregation being used. The reason is CSV Job parameters are missing the meta.objectType.keyword field which should be populated with discover at CSV job creation time. Issue: #64119

Checklist

For maintainers

@tsullivan tsullivan changed the title Reporting/telemetry status breakdown [Reporting/telemetry] Add 'statuses' object to usage to show status counts by jobType & appType Apr 21, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

@tsullivan tsullivan marked this pull request as ready for review April 21, 2020 20:48
@tsullivan
Copy link
Member Author

@elasticmachine merge upstream


type AggregationKeys = 'jobTypes' | 'layoutTypes' | 'objectTypes' | 'statusTypes';
export type AggregationResults = { [K in AggregationKeys]: AggregationBuckets } & {
export interface AggregationResultBuckets {
Copy link
Member Author

@tsullivan tsullivan Apr 21, 2020

Choose a reason for hiding this comment

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

There are a lot of Typescript changes in this PR, because the previous AggregationResults type was taking a few "shortcuts" that made it difficult to build more to it. Therefore it has been replaced with AggregationResultBuckets

jobTypes: {
terms: { field: JOB_TYPES_FIELD, size: DEFAULT_TERMS_SIZE },
aggs: {
appNames: { terms: { field: OBJECT_TYPES_FIELD, size: DEFAULT_TERMS_SIZE } }, // NOTE Discover/CSV export is missing the 'meta.objectType' field, so Discover/CSV results are missing for this agg
Copy link
Member Author

Choose a reason for hiding this comment

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

To make it clear, this aggregation is not supporting metrics for the way we store CSV reports, but all the other types of reports are supported.

See the Jest snapshot for what it looks like for CSV

Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

Overall, looks good so I'm approving to unblock. I did have a question about the statuses type and the comment describing the getAppStatuses behavior though

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@tsullivan tsullivan merged commit 6b5a965 into elastic:master Apr 27, 2020
@tsullivan tsullivan deleted the reporting/telemetry-status-breakdown branch April 27, 2020 18:49
tsullivan added a commit to tsullivan/kibana that referenced this pull request Apr 27, 2020
…ounts by jobType & appType (elastic#63922)

* [Reporting] Additional status by app data for usage

* --wip-- [skip ci]

* clean up types

* add a prettier-ignore

* fix types

* --wip-- [skip ci]

* fix typo

* more tests

* Tweak the data model

* fix the comments and type keys to reflect the data model

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
tsullivan added a commit that referenced this pull request Apr 27, 2020
…ounts by jobType & appType (#63922) (#64551)

* [Reporting] Additional status by app data for usage

* --wip-- [skip ci]

* clean up types

* add a prettier-ignore

* fix types

* --wip-- [skip ci]

* fix typo

* more tests

* Tweak the data model

* fix the comments and type keys to reflect the data model

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@sophiec20 sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes review v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants