-
Notifications
You must be signed in to change notification settings - Fork 438
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
[O11y][Kubernetes] Rally benchmark kubernetes.state_container
#9106
[O11y][Kubernetes] Rally benchmark kubernetes.state_container
#9106
Conversation
kubernetes.state_container
🚀 Benchmarks reportPackage
|
Data stream | Previous EPS | New EPS | Diff (%) | Result |
---|---|---|---|---|
apigateway_logs |
13888.89 | 11627.91 | -2260.98 (-16.28%) | 💔 |
elb_logs |
5464.48 | 3021.15 | -2443.33 (-44.71%) | 💔 |
emr_logs |
25641.03 | 18867.92 | -6773.11 (-26.42%) | 💔 |
Package mysql
👍(0) 💚(0) 💔(2)
Expand to view
Data stream | Previous EPS | New EPS | Diff (%) | Result |
---|---|---|---|---|
error |
14925.37 | 7092.2 | -7833.17 (-52.48%) | 💔 |
slowlog |
20833.33 | 13333.33 | -7500 (-36%) | 💔 |
To see the full report comment with /test benchmark fullreport
"name": "state_container" | ||
}, | ||
"event": { | ||
"duration": {{ $event_duration }}, |
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.
ingested fields is missing
eg. event.ingested: "2024-02-08T12:18:41Z"
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.
Please refer here for the same doubt I had. It's taking the time actually when the event is ingested.
packages/kubernetes/_dev/benchmark/rally/statecontainer-benchmark/template.ndjson
Show resolved
Hide resolved
"phase": "{{ $status_phase }}", | ||
"ready": {{ $status_ready }}, | ||
"restarts": {{ $restarts }}, | ||
"reason": "{{ $reason }}" |
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.
"reason": "{{ $reason }}" | |
"last_terminated_reason": "{{ $reason }}" |
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.
Took reference from here. Found this field you are mentioning as kubernetes.container.status.last_terminated_reason
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.
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.
When container is in terminated or waiting state ,the status.reason is populated
When container is in running state, then the status kubernetes.container.status.last_terminated_reason is populated
So it is probably another if case for you, that when phase is running then populate this kubernetes.container.status.last_terminated_reason
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.
@gizas Thanks for this. Added the same.
}, | ||
"id": "container-{{ $rangeofid }}", | ||
"status": { | ||
"phase": "{{ $status_phase }}", |
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.
The only values that can probably cause us problems in the future are these one for the status. Because the way it is implemented now a 'running' phase can have ready: false , because those values are randomly assigned.
Can you consider an if case like https://github.com/elastic/elastic-integration-corpus-generator-tool/blob/main/assets/templates/aws.billing/schema-b/gotext.tpl#L64 ?
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.
Sure. Updated the same. Please have a look.
@aliabbas-elastic I have tested them and work fine. Thanks for this. Added some comments. Also because the dataset is named state_container, I would also advise that all folders/paths should be renamed to |
packages/kubernetes/_dev/benchmark/rally/statecontainer-benchmark/config.yml
Outdated
Show resolved
Hide resolved
packages/kubernetes/_dev/benchmark/rally/statecontainer-benchmark/fields.yml
Show resolved
Hide resolved
@gizas Yes initially kept the name as you mentioned but facing these below error in linting
Saw couple of data streams as well with the present naming convention. Keeping the naming as per you suggested is definitely better. Is this a bug in the |
You are right. I could not find in elastic-package where this is done. in my tests: #Changing the folder to elastic-package lint
Error: linting package failed: found 1 validation error:
1. item [state_container-benchmark] is not allowed in folder [/Users/andreasgkizas/elastic/integrations/packages/kubernetes/_dev/benchmark/rally] Changing the file only and NOT the folder to
Then the command becomes: Dont you think is better? Or creates more confusion? |
Sounds better than the previous as there will be no ambiguity in running a benchmark without looking at the benchmark structure. |
a4ff42c
to
27d02ea
Compare
@aliabbas-elastic . @gizas the limit on folder naming is by folder: no limit on the scenario name: it's just a matter of changing the specs, please proceed as you see it fitting :) |
packages/kubernetes/_dev/benchmark/rally/statecontainer-benchmark/template.ndjson
Outdated
Show resolved
Hide resolved
💚 Build Succeeded
History
cc @aliabbas-elastic |
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Proposed commit message
state_container
data stream ofKubernetes
Checklist
How to test this PR locally
Run this command from package root
elastic-package benchmark rally --benchmark state_container-benchmark -v
elastic-package benchmark stream --benchmark state_container-benchmark -v
Related issues
Screenshots