Skip to content

Commit

Permalink
[EventEngine] Change TXT lookup result type to std::vector<std::strin…
Browse files Browse the repository at this point in the history
…g> (#33030)

One TXT lookup query can return multiple TXT records (see the following
example). `EventEngine::DNSResolver` should return all of them to let
the caller (e.g. `event_engine_client_channel_resolver`) decide which
one they would use.

```
$ dig TXT wikipedia.org

; <<>> DiG 9.18.12-1+build1-Debian <<>> TXT wikipedia.org
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 49626
;; flags: qr rd ra; QUERY: 1, ANSWER: 3, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 512
;; QUESTION SECTION:
;wikipedia.org.                 IN      TXT

;; ANSWER SECTION:
wikipedia.org.          600     IN      TXT     "google-site-verification=AMHkgs-4ViEvIJf5znZle-BSE2EPNFqM1nDJGRyn2qk"
wikipedia.org.          600     IN      TXT     "yandex-verification: 35c08d23099dc863"
wikipedia.org.          600     IN      TXT     "v=spf1 include:wikimedia.org ~all"
```

Note that this change also deviates us from the iomgr's DNSResolver API
which uses std::string as the result type.


<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
  • Loading branch information
yijiem committed May 8, 2023
1 parent 56938bc commit 7df0e11
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 6 deletions.
2 changes: 1 addition & 1 deletion include/grpc/event_engine/event_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ class EventEngine : public std::enable_shared_from_this<EventEngine> {
absl::AnyInvocable<void(absl::StatusOr<std::vector<SRVRecord>>)>;
/// Called with the result of a TXT record lookup
using LookupTXTCallback =
absl::AnyInvocable<void(absl::StatusOr<std::string>)>;
absl::AnyInvocable<void(absl::StatusOr<std::vector<std::string>>)>;

virtual ~DNSResolver() = default;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "absl/container/flat_hash_set.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/strip.h"
#include "absl/types/optional.h"
Expand Down Expand Up @@ -128,7 +129,7 @@ class EventEngineClientChannelDNSResolver : public PollingResolver {
void OnBalancerHostnamesResolved(
std::string authority,
absl::StatusOr<std::vector<EventEngine::ResolvedAddress>> addresses);
void OnTXTResolved(absl::StatusOr<std::string> service_config);
void OnTXTResolved(absl::StatusOr<std::vector<std::string>> service_config);
// Returns a Result if resolution is complete.
// callers must release the lock and call OnRequestComplete if a Result is
// returned. This is because OnRequestComplete may Orphan the resolver,
Expand Down Expand Up @@ -251,7 +252,7 @@ EventEngineClientChannelDNSResolver::EventEngineDNSRequestWrapper::
resolver_.get(), resolver_->name_to_resolve().c_str());
txt_handle_ = event_engine_resolver_->LookupTXT(
[self = Ref(DEBUG_LOCATION, "OnTXTResolved")](
absl::StatusOr<std::string> service_config) {
absl::StatusOr<std::vector<std::string>> service_config) {
self->OnTXTResolved(std::move(service_config));
},
absl::StrCat("_grpc_config.", resolver_->name_to_resolve()),
Expand Down Expand Up @@ -388,7 +389,7 @@ void EventEngineClientChannelDNSResolver::EventEngineDNSRequestWrapper::
}

void EventEngineClientChannelDNSResolver::EventEngineDNSRequestWrapper::
OnTXTResolved(absl::StatusOr<std::string> service_config) {
OnTXTResolved(absl::StatusOr<std::vector<std::string>> service_config) {
ValidationErrors::ScopedField field(&errors_, "txt lookup");
absl::optional<Resolver::Result> result;
{
Expand All @@ -400,7 +401,21 @@ void EventEngineClientChannelDNSResolver::EventEngineDNSRequestWrapper::
errors_.AddError(service_config.status().message());
service_config_json_ = service_config.status();
} else {
service_config_json_ = absl::StrCat("grpc_config=", *service_config);
constexpr char kServiceConfigAttributePrefix[] = "grpc_config=";
auto result = std::find_if(service_config->begin(), service_config->end(),
[&](absl::string_view s) {
return absl::StartsWith(
s, kServiceConfigAttributePrefix);
});
if (result != service_config->end()) {
service_config_json_ =
result->substr(sizeof(kServiceConfigAttributePrefix));
} else {
service_config_json_ = absl::UnavailableError(absl::StrCat(
"failed to find attribute prefix: ", kServiceConfigAttributePrefix,
" in TXT records"));
errors_.AddError(service_config_json_.status().message());
}
}
result = OnResolvedLocked();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ ThreadyEventEngine::ThreadyDNSResolver::LookupTXT(LookupTXTCallback on_resolve,
Duration timeout) {
return impl_->LookupTXT(
[this, on_resolve = std::move(on_resolve)](
absl::StatusOr<std::string> record) mutable {
absl::StatusOr<std::vector<std::string>> record) mutable {
return engine_->Asynchronously([on_resolve = std::move(on_resolve),
record = std::move(record)]() mutable {
on_resolve(std::move(record));
Expand Down

0 comments on commit 7df0e11

Please sign in to comment.