-
Notifications
You must be signed in to change notification settings - Fork 0
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
NH-37575 Move benchmark job to push workflow #240
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Added a comment about removing AO from the mix, and minor how about updating the summary wording to something like:
Run at Mon Jul 08 19:09:08 UTC 2024
release : compares no agent, latest Otel standard, and latest SWO agents
5 users, 5000 iterations
----------------------------------------------------------
| Agent | OTel | SWO | none |
| ------ | ------ | ------ | ------ |
@@ -22,7 +22,8 @@ public class Agent { | |||
new Agent("latest", "latest mainstream release", OTEL_LATEST); | |||
public static final Agent LATEST_SNAPSHOT = | |||
new Agent("snapshot", "latest available snapshot version from main"); | |||
public static final Agent NH_LATEST_RELEASE = new Agent(LatestSolarwindsAgentResolver.useAOAgent ? "AO" : "NH", "latest Solarwinds agent"); | |||
public static final Agent NH_LATEST_RELEASE = | |||
new Agent(LatestSolarwindsAgentResolver.useAOAgent ? "AO" : "NH", "latest Solarwinds agent"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're far enough along with SWO Java at this point that the AO option can be removed in this benchmark, or replace it with SWO GA (to compare current build with released one).
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
0a04974
to
34b74fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cleverchuk for enabling CodeQL too! Good to see its scan findings. A couple nitpicky comments.
.github/workflows/codeql.yml
Outdated
|
||
# Autobuild fails so use custom build steps | ||
- name: Set up JDK 17 | ||
uses: actions/setup-java@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems GHA is warning about this using deprecated Node.js version, use v4 instead?
.withEnv("APPOPTICS_TRUSTEDPATH", "/test-server-grpc.crt") | ||
.withEnv("APPOPTICS_DEBUG_LEVEL", "info") | ||
.withEnv("APPOPTICS_COLLECTOR", "AOCollector:12223") | ||
.withEnv("APPOPTICS_TRUSTEDPATH", "/test-server-grpc.crt") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, can remove?
34b74fa
to
5b758ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thanks @cleverchuk!
Tl;dr: Fix benchmark job
Context:
This PR fixes the benchmark job by removing container liveness check because it isn't working and moves it to the
Push
work flow so it runs on every push. Also adds some formatting change to keep it interesting 😁Supersedes #239
Test Plan:
Test services 0, 1 and 2