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

feat: simulations format #114

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Conversation

kkulak
Copy link
Contributor

@kkulak kkulak commented Mar 28, 2024

Description

Gatling scenarios can be defined in two ways at the moment:

  • via hardcoded simulationData
  • or via custom docker image. However, the only accepted format is Gatling Bundle, which has its own limitations. For example, we cannot define simulation in languages other than Scala, while Gatling supports Kotlin and Java ootb.

Furthermore, I'd like to make use of build tool, sth like that:

My proposal is to add a new property - simulationsFormat - which modifies the command being run by gatling-runner - it only decides whether we should use gatling.sh or gradle wrapper for the test execution. I've already checked the changed on my private EKS cluster, and they seem to work correctly, including features like S3 results upload.

Tomorrow I'll add some examples and update docs. Let me know please @kane8n @gold-kou what are your thoughts. Cheers!

Checklist

Please check if applicable

  • Tests have been added (if applicable, ie. when operator codes are added or modified)
  • Relevant docs have been added or modified (if applicable, ie. when new features are added or current features are modified)

Copy link
Contributor

@kane8n kane8n left a comment

Choose a reason for hiding this comment

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

Thanks for the great proposal!
I left some review comments, please check them out.

if [ ${SIMULATIONS_FORMAT} = "bundle" ]; then
gatling.sh -sf ${SIMULATIONS_DIR_PATH} -s ${SIMULATION_CLASS} -rsf ${RESOURCES_DIR_PATH} -rf ${RESULTS_DIR_PATH} -rm local
elif [ ${SIMULATIONS_FORMAT} = "gradle" ]; then
./gradlew -Dgatling.core.directory.results=${RESULTS_DIR_PATH} gatlingRun-${SIMULATION_CLASS}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why you don't pass the simulations or resources directories as arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with Gatling Gradle Plugin, these files do have quite typical locations:

  • simulations are under src/gatling/java or src/gatling/kotlin or src/gatling/java
  • resources are under src/gatling/resources

I see that there's an option to customize these options via:

sourceSets {
  gatling {
    scala.srcDir "folder1" <1>
    // or
    scala.srcDirs = ["folder1"] <2>

    resources.srcDir "folder2" <3>
    // or
    resources.srcDirs = ["folder2"] <4>
  }
}

but I do not know upfront in which language the simulations were written, so it's quite hard to guess. IMHO convention over configuration rule applies here perfectly :)

api/v1alpha1/gatling_types.go Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
@kkulak
Copy link
Contributor Author

kkulak commented Mar 29, 2024

@kane8n docs updated, comms addressed. Let me know please if there's anything pending :)

@kane8n
Copy link
Contributor

kane8n commented Mar 29, 2024

@kkulak
Thank you!
I think the updated one looks good.
I'll ask other members to review it, but please you'll have to wait until next week.

@kane8n kane8n requested a review from a team April 2, 2024 10:59
@kkulak
Copy link
Contributor Author

kkulak commented Apr 4, 2024

hi All, did you have a chance to look into the PR? @gold-kou @kane8n

@kane8n
Copy link
Contributor

kane8n commented Apr 5, 2024

@kkulak
It takes a little time because other members are busy. Sorry for keeping you waiting.

Copy link
Contributor

@gold-kou gold-kou left a comment

Choose a reason for hiding this comment

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

@kkulak CC: @kane8n
Sorry for being late.
To be honest, I'm not familiar with using bundle or gradle as a simulation.
However looks no regression at least.

Thx for your contribution. 🎉

Copy link
Contributor

@kane8n kane8n left a comment

Choose a reason for hiding this comment

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

lgtm!
We'll have it ready for release this week.

@kane8n kane8n merged commit ffebe49 into st-tech:main Apr 12, 2024
3 checks passed
@kane8n
Copy link
Contributor

kane8n commented Apr 12, 2024

@kkulak
Thanks for the contribute, we released it as v0.9.7!

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.

3 participants