[SYNPY-1344] Remove need to manually propogate otel context

[BryanFauble]

@BWMac Which would you prefer: The current way we are passing the context along (No change), or wrapping the calling side to handle for this (This change)?

Problem:
Because we are using run_in_executor to execute blocking code in an async context otel context is not propogating to the new code that is executing. As a result we were required to pass the context along and modify the functions to accept the parameter to make it work.

Solution:
Creating a wrapper function that we can add to run_in_executor that attaches the otel context for us; prevents us from needing to modifying all of the sync code function signatures.

Testing:
I verified I could run our OOP test scripts and all otel context remained:

[pep8speaks]

Hello @BryanFauble! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file synapseclient/client.py:

Line 1574:89: E501 line too long (92 > 88 characters)
Line 1589:89: E501 line too long (92 > 88 characters)
Line 1660:89: E501 line too long (91 > 88 characters)
Line 1692:89: E501 line too long (105 > 88 characters)
Line 1734:89: E501 line too long (106 > 88 characters)
Line 1735:89: E501 line too long (102 > 88 characters)
Line 1766:89: E501 line too long (91 > 88 characters)

• In the file synapseclient/core/utils.py:

Line 1272:89: E501 line too long (89 > 88 characters)

• In the file synapseclient/table.py:

Line 508:89: E501 line too long (90 > 88 characters)

Comment last updated at 2024-01-29 18:31:52 UTC

[sonarcloud]

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

5 New issues
0 Security Hotspots
96.9% Coverage on New Code
7.8% Duplication on New Code

See analysis details on SonarCloud

[thomasyu888]

@BryanFauble On a high level, I prefer this over passing around the context manager. I haven't had the time to review closer, but this is great.

Can the team @Sage-Bionetworks/dpe pitch in on this review?

[BWMac]

I like this change. LGTM but just one question.

[BWMac]

If we can remove this after SYNPY-1411 is complete, what does the difference between making this change now vs. later look like?

[BryanFauble]

Making the change to use this wrapper now prevents us from needing to modify the code in client.py to manually propagate the OTEL context.

Making the change later means we will have had to propagate the OTEL to the rest of the sync methods we want to call in client.py that haven't been called yet. And all those methods need to be changed back.

[BWMac]

Looks good, I really like the mixin for permissions. Only thing is permissions issues for the demo script. I'll come back and approve once I can run it.

[BWMac]

Looks good, I really like the mixin for permissions. Only thing is permissions issues for the demo script. I'll come back and approve once I can run it.