Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc(dns): more precise stdlib resolver naming #257

Merged
merged 19 commits into from
Aug 27, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
## Checklist

- [ ] I have read the [contribution guidelines](https://github.com/ooni/spec/blob/master/CONTRIBUTING.md)
- [ ] reference issue for this pull request: `REFERENCE_ISSUE_URL`
- [ ] related ooni/probe-cli pull request: `RELATED_OONI_PROBE_CLI_PULL_REQUEST_URL`
- [ ] reference issue for this pull request: <!-- add URL here -->
- [ ] related ooni/probe-cli pull request: <!-- add URL here -->
- [ ] If I changed a spec, I also bumped its version number and/or date

Location of the issue tracker: https://github.com/ooni/probe
<!-- Location of the issue tracker: https://github.com/ooni/probe -->

## Description

Expand Down
74 changes: 49 additions & 25 deletions data-formats/df-002-dnst.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ emit the dial_id when it is zero. Rest assured that the dial_id will
be unique during a measurement session.

- `engine` (`string`; optional): the specific engine used to perform
the DNS query. If omitted implies `"system"`, which means that we are
using in a way or another `getaddrinfo`. See also below for a list
the DNS query. If omitted implies `"unknown"`, which means that we are
not sure about what resolver we're using. See also below for a list
of available resolver engines.

- `failure` (`string`; nullable): if there was an error, this field is
Expand Down Expand Up @@ -92,45 +92,69 @@ the amount of time spent performing the given lookup);
measured in the moment in which `failure` is determined (`t - t0` gives you
the amount of time spent performing the given lookup);

- `transaction_id` (`int`; optional; since 2020-01-11): if present, this is the
ID of the HTTP transaction that caused this query.
- `transaction_id` (`int`; optional; since 2020-01-11): if present and nonzero,
this is the ID of the transaction that caused this query. By grouping by transaction
ID, you could observe chains of events that are logically related. For example, a
DoH lookup includes ancillary DNS lookups, TCP connects, TLS handshakes, etc. If
zero, this field just indicates we don't know the transaction ID. Before 2022-08-26,
this field was not used in production and had a different definition where the
transaction was necessarily an HTTP transaction, which was too strict.

### DNS resolver engines

The following table documents the available DNS resolver engines.

| Engine name | Description |
| :---------- | ----------- |
| system | We are using getaddrinfo |
| go | Whatever the Go stdlib uses for the current platform |
| udp | Custom DNS-over-UDP resolver |
| tcp | Custom DNS-over-TCP resolver |
| dot | Custom DNS-over-TLS resolver |
| doh | Custom DNS-over-HTTPS resolver |
| Engine name | Description |
| :------------------ | ----------- |
| unknown | We don't know what resolver we're using (added on 2022-08-27) |
| system | We are using getaddrinfo (legacy since 2022-08-27) |
| getaddrinfo | We are using getaddrinfo |
| golang_net_resolver | We are using golang's `net.Resolver` |
| go | Alias for `golang_net_resolver` (deprecated since 2022-08-27) |
| udp | Custom DNS-over-UDP resolver |
| tcp | Custom DNS-over-TCP resolver |
| dot | Custom DNS-over-TLS resolver |
| doh | Custom DNS-over-HTTPS resolver |

Since 2022-05-29 (i.e., for `ooniprobe>=3.16.0`), we explicitly use
`getaddrinfo` whenever possible and fall back to using `go` only when
`getaddrinfo` whenever possible and fall back to using `net.Resolver` only when
`CGO_ENABLED=0`, which happens when cross compiling or when who's
building OONI has passed `CGO_ENABLED=0` explicitly from the command
line See [ooni/probe-cli#765](https://github.com/ooni/probe-cli/pull/765)
and [ooni/probe-cli#766](https://github.com/ooni/probe-cli/pull/766) for
line. See [ooni/probe-cli#765](https://github.com/ooni/probe-cli/pull/765),
[ooni/probe-cli#766](https://github.com/ooni/probe-cli/pull/766), and
[ooni/probe-cli#885](https://github.com/ooni/probe-cli/pull/885) for
more details about how we choose the resolver name.

### Representing system/go resolver results
When the engine is `getaddrinfo` or `golang_net_resolver` or `go`, we
represent a lookup host operation as a single `ANY` query and include into
the results any returned `A`, `AAAA`, and `CNAME` information by faking
a reply containing such records as answers.
bassosimone marked this conversation as resolved.
Show resolved Hide resolved

As of 2022-08-23, resolutions performed by the system/go resolver are
represented in two distinct ways:
The `go` engine is a misnomer that we corrected to `golang_net_resolver`
on 2022-08-27 during the ooniprobe 3.16.0-alpha release cycle and it
has not otherwise been used by stable ooniprobe versions. Calling such an
engine `go` was confusing because it may seem to imply that we were
actually using a DNS resolver written in Go, which we're not sure about. In
bassosimone marked this conversation as resolved.
Show resolved Hide resolved
fact, it may either be `netgo` or it may be `getaddrinfo` depending on
bassosimone marked this conversation as resolved.
Show resolved Hide resolved
golang's policies for the platform we're using.
bassosimone marked this conversation as resolved.
Show resolved Hide resolved

1. all mainline experiments artificially split a lookup into two queries and replies, one
for `A` and the other for `AAAA`;
When the engine is `system`, there are three cases:

2. new step-by-step code (which eventually will become the default) represents a
system/go lookup as a single `ANY` query/reply containing all the `A`, `AAAA`, and
`CNAME` records returned by `getaddrinfo` (or by the Go resolver).
* `ooniprobe < 3.16.0` artificially split a lookup into an `A`
query and reply and an `AAAA` query and reply (which is what we've
been doing since the Measurement Kit days);

The latter approach is more correct in terms of representing which low
* `ooniprobe 3.16.0-alpha` behaved like `getaddrinfo` until 2022-08-27
when this behavior has been fixed (it's noteworthy that this behavior
only affected unreleased versions of ooniprobe);

* `ooniprobe >= 3.16.0` does not use the `system` engine.

The `ooniprobe >= 3.16.0` approach is more correct in terms of representing which low
level operations occurred, since a `getaddrinfo` is actually a single lookup
and it's not faithful to reality to fake out two lookups.
and it's not faithful to reality to fake out two lookups. We choose to change the
engine name because we changed how we represent the results of queries, moving
away form the incorrect two-fake-queries-and-replies approach.

## Answer

Expand Down