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.

Sign up for GitHub

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

Jump to bottom

Advanced DNS metrics e2e is flaky #415

Closed
nddq opened this issue May 31, 2024 · 1 comment · Fixed by #414
Closed

Advanced DNS metrics e2e is flaky #415

nddq opened this issue May 31, 2024 · 1 comment · Fixed by #414
Assignees
@nddq
Labels
area/infra Test, Release, or CI Infrastructure type/bug Something isn't working

Comments

@nddq
Copy link
Contributor

nddq commented May 31, 2024

Describe the bug
Need to investigate why sometimes the correct advanced DNS metrics not showing up during e2e
image
@nddq nddq added type/bug Something isn't working area/infra Test, Release, or CI Infrastructure labels May 31, 2024
@nddq nddq self-assigned this May 31, 2024
@nddq nddq linked a pull request May 31, 2024 that will close this issue
test(e2e): add e2e test for latency metrics in advanced scenario #414
Merged
7 tasks
@nddq
Copy link
Contributor Author

nddq commented May 31, 2024 •
edited
Loading

Advanced DNS metrics sometimes doesn't show up right away after a single curl request, but they will show up if we make a subsequent curl request. The resulted metrics have the correct count that include both requests.

@nddq nddq closed this as completed in #414 May 31, 2024
@nddq nddq mentioned this issue Jun 4, 2024
Merged
7 tasks
github-merge-queue bot pushed a commit that referenced this issue Jun 5, 2024
@nddq
fix(e2e): add an extra nslookup when testing for advanced DNS metrics (…
4712ab7 
…#438)

# Description
Ref: #415. Added for Basic DNS
metrics but missed for Adv DNS metrics
## Related Issue

If this pull request is related to any issue, please mention it here.
Additionally, make sure that the issue is assigned to you before
submitting this pull request.

## Checklist

- [ ] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [ ] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [ ] I have correctly attributed the author(s) of the code.
- [ ] I have tested the changes locally.
- [ ] I have followed the project's style guidelines.
- [ ] I have updated the documentation, if necessary.
- [ ] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes
made.

## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.
matmerr pushed a commit to matmerr/retina that referenced this issue Jul 3, 2024
@nddq @matmerr
fix(e2e): add an extra nslookup when testing for advanced DNS metrics (…
713ea73 
…microsoft#438)

# Description
Ref: microsoft#415. Added for Basic DNS
metrics but missed for Adv DNS metrics
## Related Issue

If this pull request is related to any issue, please mention it here.
Additionally, make sure that the issue is assigned to you before
submitting this pull request.

## Checklist

- [ ] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [ ] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [ ] I have correctly attributed the author(s) of the code.
- [ ] I have tested the changes locally.
- [ ] I have followed the project's style guidelines.
- [ ] I have updated the documentation, if necessary.
- [ ] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes
made.

## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nddq nddq

Labels
area/infra Test, Release, or CI Infrastructure type/bug Something isn't working
Projects
Retina Triage Board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test(e2e): add e2e test for latency metrics in advanced scenario microsoft/retina
1 participant
@nddq