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

DirectPath Misconfiguration emitted for non-DirectPath client instances in a DirectPath enabled client application #2427

Closed
mohanli-ml opened this issue Jan 26, 2024 · 1 comment · Fixed by #3019
Closed

DirectPath Misconfiguration emitted for non-DirectPath client instances in a DirectPath enabled client application #2427

mohanli-ml opened this issue Jan 26, 2024 · 1 comment · Fixed by #3019
Assignees
@diegomarquezp
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@mohanli-ml
Copy link
Contributor

Environment details

  1. GCE
  2. gax 2.41.0
  3. GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS is set

Details

In #2105 we added warning logs for DirectPath misconfiguration. The logic is that we first check if DirectPath xDS is enabled, then check if DirectPath is enabled. This could cause misleading warning logs in the case where non-DirectPath client instances in a DirectPath enabled client application. For example: a customer's application may contains a GCS client and a Bigquery client. The customer use setAttemptDirectPath to enable DirectPath in its GCS client, and set GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS to enable DirectPath xDS. In this case, this warning log will be printed in the customer's Bigquery client.
@blakeli0 blakeli0 added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jan 27, 2024
@blakeli0
Copy link
Collaborator

We should probably check if the client setting isDirectPathEnabled() is enabled first.

diegomarquezp added a commit that referenced this issue Jul 8, 2024
@diegomarquezp
fix: improve warnings for Direct Path xDS set via env
17a5dec 
Fixes #2427
@diegomarquezp diegomarquezp mentioned this issue Jul 8, 2024
Merged
@diegomarquezp diegomarquezp self-assigned this Jul 20, 2024
diegomarquezp added a commit that referenced this issue Jul 29, 2024
@diegomarquezp @lqiu96
fix: improve warnings for Direct Path xDS set via env (#3019)
7a26115 
Fixes #2427

### Context
If `GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS` is set in the environment
aiming to enable Direct Path xDS in another client, we want to just warn
the user that _if this is intended_, it can be a misconfiguration and
that the gRPCLB Direct Path setting should be enabled as well.

### Approach
As said in the context, we will _warn_ the user that it _could_ be a
misconfiguration _if_ the env var setting was meant for the client in
question. Other warn cases remain the same.

### Coverage
Due to the fact that part of the method in question is tested via a
special env var test (not detected by SonarCloud), it will show as
uncovered. See this screenshot for coverage of all tests combined

![image](https://github.com/googleapis/sdk-platform-java/assets/22083784/af24e721-6495-4030-8804-c013b14679d7)

---------

Co-authored-by: Lawrence Qiu <lawrenceqiu@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@diegomarquezp diegomarquezp

Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: improve warnings for Direct Path xDS set via env googleapis/sdk-platform-java
3 participants
@blakeli0 @diegomarquezp @mohanli-ml