Skip to content

dns: use dns specific log component, add log events to apple resolver#17833

Merged
ggreenway merged 3 commits intoenvoyproxy:mainfrom
snowp:dns-events
Aug 26, 2021
Merged

dns: use dns specific log component, add log events to apple resolver#17833
ggreenway merged 3 commits intoenvoyproxy:mainfrom
snowp:dns-events

Conversation

@snowp
Copy link
Copy Markdown
Contributor

@snowp snowp commented Aug 24, 2021

Changes the c-ares and apple DNS resolver to use their own logging component and adds additional ENVOY_LOG_EVENT logs to the apple resolver to match the c-ares one.

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

Commit Message:
Additional Description:
Risk Level: Low, only changes logging
Testing: n/a
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Snow Pettersen <snowp@lyft.com>
Augustyniak
Augustyniak previously approved these changes Aug 24, 2021
AppleDnsResolverImpl::startResolution(const std::string& dns_name,
DnsLookupFamily dns_lookup_family, ResolveCb callback) {
ENVOY_LOG(debug, "DNS resolver resolve={}", dns_name);
ENVOY_LOG_EVENT(debug, "apple_dns_started", "DNS resolution for {} started", dns_name);
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.

Suggested change
ENVOY_LOG_EVENT(debug, "apple_dns_started", "DNS resolution for {} started", dns_name);
ENVOY_LOG_EVENT(debug, "apple_dns_start", "DNS resolution for {} started", dns_name);

To keep it a noun (as with other cases in this file)

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

wrowe commented Aug 25, 2021

/retest

for pipeline flake

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17833 (comment) was created by @wrowe.

see: more, trace.

junr03
junr03 previously approved these changes Aug 25, 2021
Copy link
Copy Markdown
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.

lgtm pending @Augustyniak suggestion

Signed-off-by: Snow Pettersen <snowp@lyft.com>
@snowp snowp dismissed stale reviews from junr03 and Augustyniak via b82274d August 25, 2021 22:32
Copy link
Copy Markdown
Member

@ggreenway ggreenway left a comment

Choose a reason for hiding this comment

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

LGTM

@ggreenway ggreenway merged commit 779d53c into envoyproxy:main Aug 26, 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.

5 participants