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

Context handling in Tracers #4815

Open
c2nes opened this issue Aug 30, 2018 · 3 comments
Open

Context handling in Tracers #4815

c2nes opened this issue Aug 30, 2018 · 3 comments
Assignees
@zhangkun83
Milestone
Next

Comments

@c2nes
Copy link

c2nes commented Aug 30, 2018

In 1.10 and 1.11 the Context returned by ServerStreamTracer.filterContext was attached when streamClosed was invoked. As of 1.12 (and continuing through 1.14) this no longer seems to be the case. It appears this was undocumented behavior and relying on it was my mistake.

That said, I think it would be nice if ServerStreamTracer methods were guaranteed to be invoked with the filtered Context attached. Either way, it would be helpful to have the Javadoc for filterContext specify whether or not the filtered Context is attached when other ServerStreamTracer methods are invoked.
@zhangkun83
Copy link
Contributor

streamClosed() was never meant to be run under the context. I looked back at 1.11 code but was unable to confirm that it was the case. It needs to be clarified in the javadoc and a regression test to enforce it.

@zhangkun83 zhangkun83 self-assigned this Sep 4, 2018
@zhangkun83 zhangkun83 added this to the 1.16 milestone Sep 4, 2018
@ejona86 ejona86 modified the milestones: 1.16, 1.17 Oct 11, 2018
@creamsoup creamsoup modified the milestones: 1.17, 1.18 Dec 4, 2018
@dapengzhang0 dapengzhang0 modified the milestones: 1.18, 1.19 Jan 2, 2019
@carl-mastrangelo
Copy link
Contributor

@zhangkun83 Can this be fixed for this cycle? It seems we keep kicking it down the road.

@carl-mastrangelo
Copy link
Contributor

@zhangkun83 ping on this again

@carl-mastrangelo carl-mastrangelo modified the milestones: 1.19, 1.20 Feb 26, 2019
@zhangkun83 zhangkun83 modified the milestones: 1.20, 1.21 Apr 1, 2019
@ejona86 ejona86 modified the milestones: 1.21, v1.22 May 7, 2019
@creamsoup creamsoup modified the milestones: v1.22, v1.23 Jun 18, 2019
@dapengzhang0 dapengzhang0 modified the milestones: 1.23, Next Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zhangkun83 zhangkun83

Labels
None yet
Projects
None yet
Milestone
Next
Development

No branches or pull requests
6 participants
@c2nes @creamsoup @ejona86 @dapengzhang0 @carl-mastrangelo @zhangkun83