Skip to content

Fix cloudfunctions test#270

Merged
psx95 merged 1 commit intoGoogleCloudPlatform:mainfrom
dashpole:fix_cloudfunctions_test
Nov 16, 2023
Merged

Fix cloudfunctions test#270
psx95 merged 1 commit intoGoogleCloudPlatform:mainfrom
dashpole:fix_cloudfunctions_test

Conversation

@dashpole
Copy link
Copy Markdown
Contributor

@dashpole dashpole commented Nov 15, 2023

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 force-pushed the fix_cloudfunctions_test branch from ed4c445 to 14a1ab0 Compare November 16, 2023 15:08
@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

psx95 commented Nov 16, 2023

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
Copy Markdown
Contributor Author

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

@psx95
Copy link
Copy Markdown
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

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