Skip to content

upstream: log event for outlier detection ejection and connection draining#17879

Merged
snowp merged 5 commits intoenvoyproxy:mainfrom
snowp:outlier-logging
Aug 27, 2021
Merged

upstream: log event for outlier detection ejection and connection draining#17879
snowp merged 5 commits intoenvoyproxy:mainfrom
snowp:outlier-logging

Conversation

@snowp
Copy link
Contributor

@snowp snowp commented Aug 26, 2021

Signed-off-by: Snow Pettersen snowp@lyft.com

Risk Level: Low, additional logging
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Snow Pettersen <snowp@lyft.com>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #17879 was opened by snowp.

see: more, trace.

@snowp snowp changed the title upstream: log event when outlier detector ejects a host upstream: log event for outlier detection ejection and connection draining Aug 26, 2021
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
@snowp snowp marked this pull request as ready for review August 26, 2021 19:36
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Copy link
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

thanks for adding. Just a comment about one of the lines.

}

void ClusterManagerImpl::drainConnections(const std::string& cluster) {
ENVOY_LOG_EVENT(debug, "drain_connections_called", "drainConnections called for cluster {}",
Copy link
Member

Choose a reason for hiding this comment

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

we use the overloaded function below that drains all clusters. Because of the threading they actually don't call each other. So the log event belongs below (or in both places if you want to leave this one here too).

Signed-off-by: Snow Pettersen <snowp@lyft.com>
Copy link
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

thanks @snowp

@snowp snowp merged commit fd06e82 into envoyproxy:main Aug 27, 2021
buildbreaker pushed a commit to envoyproxy/envoy-mobile that referenced this pull request Aug 28, 2021
Pulls in several instrumentation changes that make use of `ENVOY_LOG_EVENT`:
- envoyproxy/envoy#17879
- envoyproxy/envoy#17833
- envoyproxy/envoy#17853

Signed-off-by: Jose Nino <jnino@lyft.com>
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Pulls in several instrumentation changes that make use of `ENVOY_LOG_EVENT`:
- #17879
- #17833
- #17853

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Pulls in several instrumentation changes that make use of `ENVOY_LOG_EVENT`:
- #17879
- #17833
- #17853

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.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.

2 participants