Implement http.Hijacker for otelmux#6562
Conversation
commit 9fa866688216dd5f08a22c5f3c01da3337fc400f Merge: 8b0f94e6 8a49875 Author: Pasha, Rehan <Rehan.Pasha@fmr.com> Date: Sat Sep 7 21:58:48 2024 +0530 Merge branch 'main' into fix-5402 Signed-off-by: Pasha, Rehan <Rehan.Pasha@fmr.com> commit 8b0f94e6b500c5e2d65d1c65bbcbb63087afe7e7 Author: Pasha, Rehan <Rehan.Pasha@fmr.com> Date: Sat Sep 7 21:57:41 2024 +0530 Update CHANGELOG.md Signed-off-by: Pasha, Rehan <Rehan.Pasha@fmr.com> commit f76eb1d70480828fb0626b3285a46a82cd6c13a7 Author: Pasha, Rehan <Rehan.Pasha@fmr.com> Date: Sat Sep 7 20:35:39 2024 +0530 Delete instrgen/driver/driver Signed-off-by: Pasha, Rehan <Rehan.Pasha@fmr.com> commit 158e36cae0eff704203349929c919c0d8f4e669d Author: Rehan Pasha <rehan.pasha@fmr.com> Date: Sat Sep 7 11:03:24 2024 -0400 Updating test case and lint Signed-off-by: Rehan Pasha <rehan.pasha@fmr.com> commit 256b7de Author: Rehan Pasha <rehan.pasha@fmr.com> Date: Tue Aug 27 06:28:33 2024 -0400 Adding test case for otelmux and fixing lint Signed-off-by: Rehan Pasha <rehan.pasha@fmr.com> Signed-off-by: Rehan Pasha <rehan.pasha@fmr.com>
Signed-off-by: Rehan Pasha <rehan.pasha@fmr.com>
Signed-off-by: rehanpfmr <111350825+rehanpfmr@users.noreply.github.com>
|
|
||
| router.ServeHTTP(w, r) | ||
| assert.True(t, called, "failed to run test") | ||
| assert.True(t, called) |
There was a problem hiding this comment.
Why does this test need changing?
| assert.Equal(t, 1, calledTest, "failed to run test") | ||
| } | ||
|
|
||
| func TestRecordingResponseWriterHijack(t *testing.T) { |
There was a problem hiding this comment.
Those two tests need to be fixed (so does the linter).
Also, we should be testing calling Hijack() when hijacker is implemented.
There was a problem hiding this comment.
@rehanpfmr can you add or modify the existing test to test successfully calling Hijack, upgrading the connection might be a good test
Co-authored-by: Damien Mathieu <42@dmathieu.com>
|
Sure, I have started looking into it again. |
|
Hi @rehanpfmr, I believe this should be resolved by the recent semconv refactor, the new implementation should preserve the original interfaces implemented by the writer, take a look at these PRs and feel free to test and bring up any issues. |
|
Hey @rehanpfmr, this should be resolved so I'm going to close the PR and related issue, reach out if there are any lingering issues, thanks. |
3 participants
Issue: #5402