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

Create integration tests for SPM #5608

Open
yurishkuro opened this issue Jun 12, 2024 · 3 comments
Open

Create integration tests for SPM #5608

yurishkuro opened this issue Jun 12, 2024 · 3 comments
Labels
good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Jun 12, 2024

In #5603 the bot is trying to upgrade a dependency of the example. We don't actually know if that upgrade is safe or not, it may also require argument changes.

The main issue is that this docker-compose/monitor/docker-compose.yml file is not being testing in the CI. SPM is a major functionality, it's good to have some basic smoke test that it's working, as well as to test that the corresponding docker compose file is valid. An example of such e2e test is scripts/build-all-in-one-image.sh where we validate that the resulting image is correctly serving the web UI.

Proposal for integration test:

  • perform a fresh build of Jaeger code (don't load published images), to test the current branch
  • bring up services via docker compose
  • wait for all services to respond successfully to health checks
  • for each service in microsim
    • loop until at least 3 data non-zero data points are retrieved from the REST API
    • or until the overall timeout (say 10min) is reached, in which case fail the test
    • we need to test that the response also contains the right labels, specifically to catch this issue: Restore "operation" name in the metrics response #5673

Additional improvements:

  • The prometheus config is set to scrape metrics every 15sec, but in the UI the data points come with granularity of 1min. That means the test will have to run for at least 3min before succeeding, an artificial delay. We should find a way to use shorter interval (it may be already supported by the query API)
  • Check not just calls metric, but also errors, at least for services that may have errors
    • This might require improvement to microsim, I am not sure if it supports generating errors.
@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Jun 12, 2024
@EraKin575
Copy link
Contributor

Hi, @yurishkuro ! I am new contributor to this project! I would love to take this up!

@kunal-511
Copy link

Hi, I have made some changes according to this issue's requirements, I am ready to make a PR request. @yurishkuro Please assign me.

@yurishkuro
Copy link
Member Author

@kunal-511 Feel free to open pr. We don't assign issues.

yurishkuro added a commit that referenced this issue Jun 19, 2024
## Which problem is this PR solving?
- Part of #5608
- SPM Makefile was calling a target that was removed in previous
refactoring

## Description of the changes
- Build the image directly instead of calling much more expensive target
- Use workflow added in #5640 to run the build
- Change crossdock target to not build debug image with devle, which is
expensive

## How was this change tested?
- `make build && make dev`

---------

Signed-off-by: mehul gautam <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Co-authored-by: mehul gautam <[email protected]>
yurishkuro added a commit that referenced this issue Jun 24, 2024
## Which problem is this PR solving?
- #5608

## Description of the changes
- changelog: added basic build test for SPM

## How was this change tested?
- manual testing and CI

## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: mehul gautam <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: mehul gautam <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Co-authored-by: Raghuram Kannan <[email protected]>
Co-authored-by: mehul gautam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Development

No branches or pull requests

3 participants