RUM-5551 Add Benchmark Tests Project #1977
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maxep
force-pushed
the
feature/continuous-benchmarks
branch
9 times, most recently
from
eac66a0
to
a6ad807
Compare
cswatt
added
the
editorial review
|
added DOCS-8608 to review |
mariedm
reviewed
Aug 2, 2024
...markTests/BenchmarkTests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Outdated
Show resolved
Hide resolved
| var window: UIWindow? | ||
|
|
||
| func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool { | ||
| let info = try! TestInfo() // crash if test info are missing or malformed |
There was a problem hiding this comment.
Do we want to log an error here for debugging purposes?
There was a problem hiding this comment.
The exception error will give us enough info to troubleshoot 👍
There was a problem hiding this comment.
I imagine real implementations for export, flush and shutdown will come later?
There was a problem hiding this comment.
Yes, implementation will come later 👍
maxep
added
editorial review
maxep
force-pushed
the
feature/continuous-benchmarks
branch
from
a6ad807
to
bc24e9d
Compare
ncreated
reviewed
Aug 6, 2024
ncreated
requested changes
Aug 6, 2024
There was a problem hiding this comment.
Great start for continuous benchmarking project 🚀! I really like the work done on shell scripts, CI secrets and makefiles - notably reusing and sharing existing bits. This PR greatly follows conventions started with GitLab migration 🏅.
I left few comments, notably touching on two areas:
- Benchmark project configuration: we should aim at providing most of it through
xcconfigfiles rather than utilizingpbxprojoverrides (this is hard to maintain). - Level of details in
README.md- this file doesn't look future-proof in its current state. Details will change, butREADME.mdwill be likely forgotten. Also, it doesn't cover enough details for setting up s8s tests and I don't even think they should belong to this file. We must document all aspects critical to team's operations, likely in iOS > Automations > Benchmark Tests Confluence page (can be done later, ofc). 👈 This is a lesson learnt from Shopist iOS and E2E Tests automations, which both includeREADME.mdbut working on s8s tests requires reverse-engineering of existing setup. For that reason we should clearly separate what we want from README (high-level overview for public audience) vs Confluence (internal details for the team).
maxep
force-pushed
the
feature/continuous-benchmarks
branch
from
bc24e9d
to
d76d2a2
Compare
ncreated
previously approved these changes
Aug 8, 2024
| pgrep -q Xcode && killall Xcode && echo_warn "- Xcode killed" || echo_succ "- Xcode not running" | ||
| sleep 0.5 && echo "- launching" # Sleep, otherwise, if Xcode was running it often fails with "procNotFound: no eligible process with specified descriptor" | ||
| open --env DD_TEST_UTILITIES_ENABLED "$TEST_WORKSPACE" | ||
| open --new --env DD_TEST_UTILITIES_ENABLED "$TEST_WORKSPACE" |
Co-authored-by: Maciek Grzybowski <[email protected]>
maxep
requested review from
ncreated and
rtrieu
and removed request for
rtrieu
maxep
removed
the
editorial review
ncreated
approved these changes
Aug 14, 2024
mariedm
approved these changes
Aug 14, 2024
mariedm
reviewed
Aug 14, 2024
|
|
||
| ### Synthetics Configuration | ||
|
|
||
| Please refer to [Confluence page (internal)](https://datadoghq.atlassian.net/wiki/spaces/RUMP/pages/3981476482/Benchmarks+iOS) |
rtrieu
approved these changes
Aug 14, 2024
|
|
||
| ## CI | ||
|
|
||
| CI continuously builds, signs, and uploads a runner application to Synthetics which runs predefined tests. |
There was a problem hiding this comment.
Suggested change
| CI continuously builds, signs, and uploads a runner application to Synthetics which runs predefined tests. | |
| CI continuously builds, signs, and uploads a runner application to Synthetics, which runs predefined tests. |
There was a problem hiding this comment.
Thank you, adding in a follow-up PR 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What and why?
Introduce the Benchmark project for continuously measure the SDK performances in a host application. This PR includes the following:
How?
Benchmark Tests Project
The
BenchmarkTests/BenchmarkTests.xcodeprojincludes a Runner application with simple logic the select a scenario from env variable. The project defines the same abstraction asE2ETeststo define scenarios.The
BenchmarkTests/Benchmarks/Package.swiftdepends onopentelemetry-swiftand will later define metrics collector and exporter to Datadog.CI
CI configuration is almost identical as
E2ETeststo build, sign, and upload the api to s8s.The added
tools/utils/code-sign.shallow sharing the steps to install the provisioning profile and certificate on the build machine.Review checklist