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

docker compose examples refactor #4590

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

olegbespalov
Copy link
Contributor

@olegbespalov olegbespalov commented Feb 21, 2025

What?

These PRs refactor how the docker-compose example is structured. So instead of the being presented in a root folder, we move it under the examples (this aligns with the way how other Grafana projects do, e.g. tempo)

It also simplifies InfluxV1 example a bit.

And it moves the OpenTelemetry example from the https://github.com/grafana/xk6-output-opentelemetry repository, since soonish it's going to be archived.

Why?

docker-compose examples' is a nice way to quickly showcase our users the integration, so having and maintaining them we're more user friendly

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the k6-documentation: grafana/k6-docs#PR-NUMBER
  • I have updated the TypeScript definitions: grafana/k6-DefinitelyTyped#PR-NUMBER
  • I have updated the release notes: link

Related PR(s)/Issue(s)

@olegbespalov olegbespalov self-assigned this Feb 21, 2025
@olegbespalov olegbespalov requested a review from a team as a code owner February 21, 2025 15:13
@olegbespalov olegbespalov requested review from mstoykov and inancgumus and removed request for a team February 21, 2025 15:13
@olegbespalov olegbespalov changed the title Feat/docker compose refactor docker compose examples refactor Feb 21, 2025
mstoykov
mstoykov previously approved these changes Feb 24, 2025
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

LGTM in general. I have left comments to bump versions of projects up a bit.

I am not certain I really want us to have dashboards inside of k6. As I've pointed before this means that users come to us to make changse to them . So we have breaking changes all the time on a thing that users should likely customize on their own, but now potentially will be surprised that their dashboards keep changing.

If the idea is that we will NOT update them all that much and keep them as an example that is fine. But IMO that should be explicitly pointed out, that we do not really want any contributions, and that users should be copying those and customizing the copies.

@olegbespalov
Copy link
Contributor Author

olegbespalov commented Feb 24, 2025

I am not certain I really want us to have dashboards inside of k6. As I've pointed before this means that users come to us to make changse to them . So we have breaking changes all the time on a thing that users should likely customize on their own, but now potentially will be surprised that their dashboards keep changing.

If the idea is that we will NOT update them all that much and keep them as an example that is fine. But IMO that should be explicitly pointed out, that we do not really want any contributions, and that users should be copying those and customizing the copies.

Like I said internally, but also posting my rational here

I believe the primary reason for having the these docker-compose (like any docker-compose in any project), dashboard examples for me is to provide a fast start to playing with technologies. They don't need to be production ready, so the simpler they are, the better they are. And having them should help our users adopt k6 better.

If the idea is that we will NOT update them all that much and keep them as an example that is fine. But IMO that should be explicitly pointed out, that we do not really want any contributions, and that users should be copying those and customizing the copies.

I believe it depends on the contribution, if the contribution is the fix because the latest k6 release introduced some breaking changes, but we forgot to update, to reflect the change,

Copy link
Member

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

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

Good idea 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants