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

Fix cloudfunctions test #270

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Fix cloudfunctions test #270

merged 1 commit into from
Nov 16, 2023

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Nov 15, 2023 •
edited
Loading

Fixes #268

It uses a synchronous export of spans to ensure spans are delivered before cloud functions reclaims the CPU.

@dashpole dashpole force-pushed the fix_cloudfunctions_test branch 3 times, most recently from bee06ea to ed4c445 Compare November 16, 2023 15:04
@dashpole dashpole requested a review from psx95 November 16, 2023 15:16
@dashpole dashpole added the bug Something isn't working label Nov 16, 2023
@dashpole dashpole marked this pull request as ready for review November 16, 2023 15:16
@dashpole dashpole requested a review from a team as a code owner November 16, 2023 15:16
@psx95
Copy link
Contributor

psx95 commented Nov 16, 2023

The reasoning behind this change makes sense to me, but I'm still not a 100% sure why the resource detector test passes. It has the same timeout and consistently passed using the same BatchSpanExporter.

Any thoughts on it ?

@dashpole
Copy link
Contributor Author

My theory was that maybe it had something to do with test ordering? If the basic test always runs last, that could be why it is the one that has the CPU taken away from it.

@psx95
Copy link
Contributor

psx95 commented Nov 16, 2023 •
edited
Loading

My theory was that maybe it had something to do with test ordering? If the basic test always runs last, that could be why it is the one that has the CPU taken away from it.

No, I think basicTest is always the first to run as per the logs. Do you think increasing the timeout along with this change would be better to improve flakiness ?

@dashpole
Copy link
Contributor Author

we can see... It seems to consistently pass with this change

@psx95
Copy link
Contributor

psx95 commented Nov 16, 2023

we can see... It seems to consistently pass with this change

Ok, I think I'm ok to merge this. If the flakiness continues we can dig deeper.

@psx95 psx95 merged commit 95082aa into GoogleCloudPlatform:main Nov 16, 2023
11 checks passed
@psx95 psx95 mentioned this pull request Nov 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psx95 psx95 psx95 approved these changes

Assignees

@psx95 psx95

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ops-java-e2e-cloud-functions-gen2 check is continuously failing for all PRs
2 participants
@dashpole @psx95