Skip to content

Egress support v2#157

Merged
RomanDzhabarov merged 14 commits intomasterfrom
egress_support_v2
Oct 18, 2016
Merged

Egress support v2#157
RomanDzhabarov merged 14 commits intomasterfrom
egress_support_v2

Conversation

@RomanDzhabarov
Copy link
Copy Markdown
Member

Work is in progress for this item. Need to add tests etc.

* Http Tracing can be enabled/disabled on a per connection manager basis.
* Here we specify some specific for connection manager settings.
*/
struct TracingConnectionManagerConfig {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't need to be in public includes. I don't think TracingType does either.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, moving that out.

* @return true if tracing is enabled otherwise it returns false;
*/
virtual bool isTracing() PURE;
virtual bool isTracing(const Http::AccessLog::RequestInfo& request_info) PURE;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename shouldTraceRequest(...)

/**
* TODO: comment.
*/
virtual const Optional<Tracing::TracingConnectionManagerConfig>& tracingInfo() PURE;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename tracingConfig()

}

// This should not happen as switch above covers all cases, this is just to make compiler happy.
throw EnvoyException("unknown tracing type");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the tracing_type value to the exception?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use NOT_IMPLEMENTED or something clear that it should not happen

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll switch to NOT_IMPLEMENTED here and refine the comment to emphasize that compiler also enforces all types covered by switch above.

data to the :ref:`configured tracing provider <config_tracing>`. Defaults to false.
.. _config_http_conn_man_tracing:

tracing
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally a sub-level json object gets its own section (see other docs). Can you make a new section on this page, put the nested json docs there, and link to it from here (again see other docs)


namespace Tracing {

class TracingContext {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

// Trace all traceable requests.
All,
// Trace only when there is an upstream failure reason.
UpstreamFailureReason
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this UpstreamFailure

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@RomanDzhabarov RomanDzhabarov merged commit a212866 into master Oct 18, 2016
@RomanDzhabarov RomanDzhabarov deleted the egress_support_v2 branch October 18, 2016 00:31
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
PiotrSikora added a commit to PiotrSikora/envoy that referenced this pull request Aug 27, 2019
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
PiotrSikora added a commit to PiotrSikora/envoy that referenced this pull request Feb 22, 2020
* Cache and share the base Wasm (envoyproxy#387)

Cache and share the base Wasm.  Use the new definition of Wasm Key to
find the base Wasm and thread local Wasm.

Signed-off-by: John Plevyak <jplevyak@gmail.com>

* Only call proxy_on_vm_start() when the VM is actaully starting. (envoyproxy#400)

Signed-off-by: John Plevyak <jplevyak@gmail.com>

* Fix SEGV when reusing the base vm. (envoyproxy#413)

Signed-off-by: John Plevyak <jplevyak@gmail.com>

Co-authored-by: John Plevyak <jplevyak@gmail.com>
wolfguoliang pushed a commit to wolfguoliang/envoy that referenced this pull request Jan 23, 2021
zh-translation: /intro/arch_overview/other_features/compression/libraries.rst
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Signed-off-by: Jose Nino jnino@lyft.com

Description: fix small typo
Risk Level: low -- fix inline comment typo

Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Signed-off-by: Jose Nino jnino@lyft.com

Description: fix small typo
Risk Level: low -- fix inline comment typo

Signed-off-by: JP Simard <jp@jpsim.com>
arminabf pushed a commit to arminabf/envoy that referenced this pull request Jun 5, 2024
mathetake added a commit that referenced this pull request Mar 3, 2026
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants