Refactor Lightstep code#724
Merged
Merged
Conversation
Member
RomanDzhabarov
left a comment
RomanDzhabarov
left a comment
There was a problem hiding this comment.
nice! One comment, and i'll check in details in a bit
| } | ||
| } | ||
|
|
||
| LightStepDriver::TlsLightStepTracer::TlsLightStepTracer(lightstep::Tracer tracer, |
Member
There was a problem hiding this comment.
Ideally, can you also move LS related tests (driver/etc) into separate test files?
Member
Author
There was a problem hiding this comment.
done. LMK if anything else needs to be refactored.
Member
Author
|
Yes I realized the same last night. Will push out another commit to
refactor the tests later today.
On Sat, Apr 8, 2017 at 12:55 PM Roman Dzhabarov ***@***.***> wrote:
***@***.**** commented on this pull request.
nice! One comment, and i'll check in details in a bit
------------------------------
In source/common/tracing/http_tracer_impl.cc
<#724 (comment)>:
> -
- Http::MessagePtr message = Grpc::Common::prepareHeaders(driver_.cluster()->name(),
- lightstep::CollectorServiceFullName(),
- lightstep::CollectorMethodName());
-
- message->body() = Grpc::Common::serializeBody(std::move(request));
-
- uint64_t timeout =
- driver_.runtime().snapshot().getInteger("tracing.lightstep.request_timeout", 5000U);
- driver_.clusterManager()
- .httpAsyncClientForCluster(driver_.cluster()->name())
- .send(std::move(message), *this, std::chrono::milliseconds(timeout));
- }
-}
-
-LightStepDriver::TlsLightStepTracer::TlsLightStepTracer(lightstep::Tracer tracer,
Ideally, can you also move LS related tests (driver/etc) into separate
test files?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#724 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AH0qd_UFB_EG9rqV5_T8r4hqQ56VlR-kks5rt7wSgaJpZM4M3lAb>
.
--
~shriram
|
rshriram
pushed a commit
to rshriram/envoy
that referenced
this pull request
Oct 30, 2018
* change bucket * do ver in advance * debug * del version * del added line * del add-on * retest * shorten name * change bucket back * reapply original change * add OWNERS to root of proxy
jpsim
pushed a commit
that referenced
this pull request
Nov 28, 2022
We should be registering the application for lifecycle callbacks in both the typed config and YAML config cases. Related to https://github.com/lyft/envoy-mobile/pull/717/files. Signed-off-by: Michael Rebello <me@michaelrebello.com> Signed-off-by: JP Simard <jp@jpsim.com>
jpsim
pushed a commit
that referenced
this pull request
Nov 29, 2022
We should be registering the application for lifecycle callbacks in both the typed config and YAML config cases. Related to https://github.com/lyft/envoy-mobile/pull/717/files. Signed-off-by: Michael Rebello <me@michaelrebello.com> Signed-off-by: JP Simard <jp@jpsim.com>
mathetake
added a commit
that referenced
this pull request
Mar 3, 2026
**Description** Previously, unknown path was responded as an internal error as opposed to the fact that it's an 404 with the user input root cause. This fixes the extproc code that way, now that users will be able to know what's wrong with the operation instead of getting the cryptic 500 error. **Related Issues/PRs (if applicable)** Contributes to #810 Closes #724 --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors lightstep driver out of http_tracer_impl into a separate file. This will simplify addition of zipkin driver ( #700 )
There is no functional change in this PR.
cc @RomanDzhabarov @fabolive