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

Enable charts to be included in output report #650

Merged

Conversation

nklincoln
Copy link
Contributor

This PR provides enhanced report outputs for a user

Breaking changes:

  • monitor configuration for docker and process monitors.
    • docker monitor name filter changed to container, to better reflect what it is filtering
    • process monitor now has the array of process to monitor under a processes block

Other Changes:

  • Main report styling change
  • Conversion of report tables to be normalised to a single metric. Each column is normalised to the largest value, to a user configurable precision.
  • Addition of charts to output report (user opt-in, for each monitor)

Signed-off-by: [email protected] [email protected]

@nklincoln nklincoln requested a review from aklenik November 14, 2019 17:20
@nklincoln nklincoln mentioned this pull request Nov 14, 2019
@nklincoln nklincoln force-pushed the enable-report-charting branch 2 times, most recently from 97dbd31 to c3a8061 Compare November 15, 2019 10:28
@nklincoln nklincoln force-pushed the enable-report-charting branch from c3a8061 to e709c4c Compare November 22, 2019 09:08
@aklenik aklenik added the PR/under review The PR is currently under review label Nov 26, 2019
Copy link
Contributor

@aklenik aklenik left a comment

Choose a reason for hiding this comment

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

Charting should be enabled, for example, for phase 4 of the Fabric CI test

* @param {string} callingMonitor the calling class name used to create a UUID
* @param {string} testLabel the current test label, required for creating a UUID
* @param {string[]} includeMetrics the metrics to include in the chart
* @param {Map<string, string>} resourceStats all resource stats
Copy link
Contributor

Choose a reason for hiding this comment

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

Param type should be Map<string, string>[]

@aklenik aklenik self-assigned this Nov 27, 2019
@aklenik aklenik added the PR/change requested The PR is blocked until the requested changes are applied label Nov 27, 2019
- Breaking changes in monitor configuration for docker and process monitors
- Report styling change
- Addition of charts to output report (opt in per monitor)
- Conversion of report tables to be normalized to a single metric
- Report presicion now a user config item

Signed-off-by: [email protected] <[email protected]>
@nklincoln nklincoln force-pushed the enable-report-charting branch from e709c4c to 3fcec80 Compare November 28, 2019 10:15
@aklenik aklenik removed PR/change requested The PR is blocked until the requested changes are applied PR/under review The PR is currently under review labels Nov 29, 2019
Copy link
Contributor

@aklenik aklenik left a comment

Choose a reason for hiding this comment

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

LGTM 💹

@aklenik aklenik merged commit e11763c into hyperledger-caliper:master Nov 29, 2019
@nklincoln nklincoln deleted the enable-report-charting branch January 8, 2020 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants