Skip to content

[Reporting] Additional CSV job output metrics#124600

Merged
tsullivan merged 2 commits intoelastic:mainfrom
tsullivan:reporting/event-log-enhancements
Feb 11, 2022
Merged

[Reporting] Additional CSV job output metrics#124600
tsullivan merged 2 commits intoelastic:mainfrom
tsullivan:reporting/event-log-enhancements

Conversation

@tsullivan
Copy link
Copy Markdown
Member

@tsullivan tsullivan commented Feb 3, 2022 •
edited
Loading

Summary

Depends on #124762

Addresses the CSV part of #121468

This PR adds job metrics which are used for:

  • Job info panel in Stack Management > Reporting
  • Telemetry: understanding the scale of reporting in the field
  • Monitoring of Reporting: understanding how long it takes to generate a report as a function of what it consists of

Checklist

Delete any items that are not applicable to this PR.

TODO

Screenshots

  1. CSV rows in the Job Info panel:
    image

@tsullivan tsullivan added zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes Team:AppServicesUx v8.2.0 labels Feb 4, 2022
@tsullivan tsullivan changed the title [Reporting] Additional job output metrics [Reporting] Additional CSV job output metrics Feb 4, 2022
@tsullivan tsullivan mentioned this pull request Feb 4, 2022
Closed
@tsullivan tsullivan force-pushed the reporting/event-log-enhancements branch from 7b26c84 to 5a94276 Compare February 10, 2022 20:02
@tsullivan tsullivan force-pushed the reporting/event-log-enhancements branch from 5a94276 to aa9332c Compare February 10, 2022 20:03
@tsullivan tsullivan marked this pull request as ready for review February 10, 2022 20:04
@tsullivan tsullivan requested review from a team as code owners February 10, 2022 20:04
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 10, 2022

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

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 10, 2022

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

@elastic elastic deleted a comment from kibana-ci Feb 10, 2022
tsullivan
tsullivan commented Feb 10, 2022
View reviewed changes
Comment thread x-pack/plugins/reporting/server/lib/event_logger/types.ts
id?: string; // "immediate download" exports have no ID
jobType: string;
byteSize?: number;
csvRows?: number;
Copy link
Copy Markdown
Member Author

@tsullivan tsullivan Feb 10, 2022

Choose a reason for hiding this comment

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

I really wonder if this should be:

csv?: { rows: number; };

We could have more CSV-specific metrics: number of columns in the CSV is one. But if there are other settings that affect performance, we should think of adding them as well.

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Feb 11, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
reporting 59.2KB 59.4KB +193.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
reporting 38.6KB 38.7KB +113.0B

History

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

dokmic
dokmic reviewed Feb 11, 2022
View reviewed changes
Comment thread x-pack/plugins/reporting/server/lib/tasks/execute_report.ts
docOutput.content_type = output.content_type || unknownMime;
docOutput.max_size_reached = output.max_size_reached;
docOutput.csv_contains_formulas = output.csv_contains_formulas;
docOutput.csv_rows = output.csv_rows;
Copy link
Copy Markdown
Contributor

@dokmic dokmic Feb 11, 2022

Choose a reason for hiding this comment

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

I think it is missing in x-pack/plugins/reporting/server/lib/store/mapping.ts.

Copy link
Copy Markdown
Member Author

@tsullivan tsullivan Feb 11, 2022

Choose a reason for hiding this comment

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

We have discussed moving the location csv_rows in the interface. Since we are not aggregating on the value yet, not having it in the mapping makes sense for now. :)

dokmic
dokmic approved these changes Feb 11, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@dokmic dokmic left a comment

Choose a reason for hiding this comment

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

Great job 👍 We will add the mapping later along with other metrics from #121468.

@tsullivan tsullivan merged commit d583a5c into elastic:main Feb 11, 2022
@tsullivan tsullivan deleted the reporting/event-log-enhancements branch February 11, 2022 17:17
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 15, 2022
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 15, 2022

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 124600 or prevent reminders by adding the backport:skip label.

@spalger spalger added the backport:skip This PR does not require backporting label Feb 15, 2022
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dokmic dokmic dokmic approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.2.0 zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tsullivan @elasticmachine @kibana-ci @kibanamachine @dokmic @spalger