Skip to content

LOG-6084: Replace functional-benchmark plot library#2992

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:masterfrom
jcantrill:log6084_benchmarker_refactor
Jul 24, 2025
Merged

LOG-6084: Replace functional-benchmark plot library#2992
openshift-merge-bot[bot] merged 1 commit intoopenshift:masterfrom
jcantrill:log6084_benchmarker_refactor

Conversation

@jcantrill
Copy link
Copy Markdown
Contributor

@jcantrill jcantrill commented Mar 18, 2025

Description

This PR:

  • replaces the functional-benchmark plot library in support of testing LOG-6084
  • Modifies the http receiver to do some of the work previously done post processing
  • Improves the report generation performance
  • Removes console report
  • Modifies the lost message graph to be a percentage

Example of results https://github.com/jcantrill/collector-experimental/blob/release-6.2/6.2.0/10m-01-5000lps-0512b-01/readme.md

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Mar 18, 2025

@jcantrill: This pull request references LOG-6084 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.8.0" version, but no target version was set.

Details

In response to this:

Description

This PR:

  • replaces the functional-benchmark plot library in support of testing LOG-6084
  • Modifies the http receiver to do some of the work previously done post processing
  • Improves the report generation performance

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 18, 2025
@openshift-ci openshift-ci Bot requested review from Clee2691 and alanconway March 18, 2025 20:22
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Mar 18, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcantrill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2025
@jcantrill jcantrill force-pushed the log6084_benchmarker_refactor branch from 28d7337 to 9e31a7a Compare March 18, 2025 21:36
@jcantrill
Copy link
Copy Markdown
Contributor Author

/test unit

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Mar 19, 2025

@jcantrill: This pull request references LOG-6084 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.8.0" version, but no target version was set.

Details

In response to this:

Description

This PR:

  • replaces the functional-benchmark plot library in support of testing LOG-6084
  • Modifies the http receiver to do some of the work previously done post processing
  • Improves the report generation performance
  • Removes console report
  • Modifies the lost message graph to be a percentage

Example of results https://github.com/jcantrill/collector-experimental/blob/release-6.2/6.2.0/10m-01-5000lps-0512-01/readme.md

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jcantrill
Copy link
Copy Markdown
Contributor Author

/test unit

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Mar 19, 2025

@jcantrill: This pull request references LOG-6084 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.8.0" version, but no target version was set.

Details

In response to this:

Description

This PR:

  • replaces the functional-benchmark plot library in support of testing LOG-6084
  • Modifies the http receiver to do some of the work previously done post processing
  • Improves the report generation performance
  • Removes console report
  • Modifies the lost message graph to be a percentage

Example of results https://github.com/jcantrill/collector-experimental/blob/release-6.2/6.2.0/10m-01-5000lps-0512b-01/readme.md

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jcantrill jcantrill force-pushed the log6084_benchmarker_refactor branch from 9e31a7a to 138de9a Compare March 19, 2025 17:30
@jcantrill
Copy link
Copy Markdown
Contributor Author

/test unit

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 29, 2025
@jcantrill jcantrill force-pushed the log6084_benchmarker_refactor branch from 138de9a to 1073690 Compare March 31, 2025 14:05
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2025
@Clee2691
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2025
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 31, 2025
@jcantrill jcantrill force-pushed the log6084_benchmarker_refactor branch from 1073690 to 34c9b7a Compare April 21, 2025 20:46
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 21, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 21, 2025
)

var (
log = utils.InitLogger("functional-benchmarker")
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.

Why aren't you using the InitStaticLogger() function here when you use it in previous files?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one should be static because it would initialize the logger for the entire process that begin in main.


// HACK - This command is for development use only
func main() {
utils.InitLogger("functional-benchmarker")
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.

Another instance of the logger?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing this since we are initializing one with var

)

var (
log = utils.InitLogger("gonumplut")
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.

InitStaticLogger()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe this should not be static as we do not want to replace the logger for the entire process. This is similar to creating a logger for the package as part of the runtime controllers. I'm not fully onboard with how loggers should be treated.

)

var (
log = utils.InitLogger("cluster")
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.

InitStaticLogger()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same here as this becomes a package level logger..

)

var (
log = utils.InitLogger("stats")
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.

InitStaticLogger()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same

@jcantrill jcantrill force-pushed the log6084_benchmarker_refactor branch from 34c9b7a to eed7f2c Compare July 15, 2025 19:48
@vparfonov
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2025
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 24, 2025

@jcantrill: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot Bot merged commit ec067fb into openshift:master Jul 24, 2025
8 checks passed
@jcantrill jcantrill deleted the log6084_benchmarker_refactor branch July 24, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. release/6.4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants