This change clarifies the naming and role of the `proxy` (née transparency)
module. There are no functional changes.
`proxy::tcp::Proxy` has been renamed to `proxy::tcp::Forward` to help
disambiguate terminology: TCP connections may be _forwarded_
by the proxy server.
Currently, the layered service implementations that comprise the HTTP
stack are a mix of `Service` and `NewService` types. In the
endpoint-specific stack. `transparency::Client` is the only layer that
actually needs to be a `NewService` if it is wrapped immediately with a
`Reconnect`.
This allows us to remove several `NewService` implementations.
This extracts a new `svc::Reconnect` middleware from `bind`, handling
connection error logging and hiding `tower_reconnect::Error` from outer
layers.
Furthermore, two HTTP/1-specific middlewares have been moved outside of
the TLS rebinding layer, since they are not dependent on TLS
configuration.
Finally, `bind`'s type aliases have been simplified, removing the
`HttpRequest` and `HttpResponse` aliases. By removing these, and
removing `transparency::Client`'s dependency on the telemetry body
types, it should be easier to change type signatures going forward.
`bind::BoundService` wraps a `Reconnect` service and handles its Connect
errors. However, `BoundService` exposes `Reconnect`'s Error type to
callers even though these errors can never be returned.
Furthermore, `Reconnect` is allowed be polled after returning an error,
triggering the inner service to be rebuilt. We needlessly duplicate this
logic in `BoundService`.
Before splitting this file up into smaller chunks, let's update
`BoundService` to more narrowly adhere to `Reconnect`s API:
- Only the inner error type is returned. `unreachable!` assertions
have been made where error variants cannot be returned.
- Do not "rebind" the stack explicitly. Instead, let `Reconnect` do
this.
- Now BoundService::call may panic if invoked before poll_ready. It's a
programming error, since `Reconnect` requires that `poll_ready` be
called first.
The `control::destination` exposes an important trait, `Bind`, that
abstracts the logic of instantiating a new service for an individual
endpoint (i.e., in a load balancer).
This interface is not specific to our service discovery implementation,
and can easily be used to model other types of client factory.
In the spirit of consolidating our HTTP-specific logic, and making the
core APIs of the proxy more visible, this change renames the `Bind`
trait to `NewClient`, simplifies the trait to have fewer type
parameters, and documents this new generalized API.
Following #84, the `telemetry::transport` module can be moved into the
`transport` module.
This should allow us to simplify type signatures by combining redundant
types. It's also hoped that we can reduce the API boilerplate around
metrics so it's much easier to instrument and track new metrics in
transport code.
The `metrics!` macro is currently local to the telemetry module.
Furthermore, the `telemetry::metrics` module no longer has
proxy-specific logic.
This change moves the `telemetry::metrics` module into a new crate,
`linkerd2_metrics`.
This will enable unifying `telemetry::http` and `telemetry::transport`
into `http` and `transport`, respectively.
The metrics Service implementation that renders prometheus metrics can
be used independently of any specific listener.
This change moves the binding and listening details into the `control`
module, as this seems like the best umbrella for the specifics of
serving things to the control plane.
Now that transport details have been separated into modules, the
`metrics::Root` type makes more sense as a `telemetry::Report` type.
With this change, the `telemetry::metrics` module holds only the
abstract structural details of metrics reporting.
To this end:
- `metrics::Root` is now `telemetry::Report`
- `metrics::Serve` is now generic over `FmtMetrics`. It's only an
implementation detail that the `telemetry::Report` type is used.
- all _Report_ types now implement `Clone` so that the main report
can be cloned for each connection (i.e. from prometheus).
The metrics server is responsible for evicting unused metrics, which
seems like an unnecessary coupling with the storage implementation.
This change moves this logic into `telemetry::http` so that the
eviction strategy is specific to the implementation.
Now that the metrics structure is shared internally to `http`,
`Report`'s implementation of `FmtMetrics` can evict expired metrics.
There are no functional changes.
Previously, much of `telemetry::http`'s types and internal
implementation details are exposed to the rest of the telemetry system.
In preparation for further changes to support more granular locking,
this change makes metric storage and recording implementation details.
Following this, `telemetry::http` exposes a `Report` type for printing
metrics to the server and a `Sensors` type used to instrument stacks
with HTTP telemetry. These types share an internally-mutable metrics
registry that is private to the http module.
The `event` types continue to be exposed to support Tap, but the
convenience exports have been removed.
The `metrics::Root` type no longer needs to be shareable. This type will
be replaced in a followup change.
In preparation for further simplifications to HTTP telemetry, this
change consolidates all HTTP-specific logic under the `telemetry::http`
module.
Specifically, the following modules have been moved:
- `telemetry::event`;
- `telemetry::metrics::labels`;
- `telemetry::metrics::record`;
- `telemetry::sensors`; and
- `telemetry::sensors::http`.
This change takes pains to avoid changing any implementation details, so
some types and methods have been made public temporarily while the
interface boundaries are not well defined. This will be fixed in a
subsequent change.
`Bind` was initially written so that a `Sensors` implementation is
optional. Over time, this hasn't proven to be very useful.
In preparation for further changes to HTTP telemetry, this change
simplifies the Bind and Sensors types so that an HTTP sensor is always
required to construct `Bind`.
Test-only constructors have been added to satisfy the case where Sensors
were omitted.
Previousy, transport telemetry was recorded by emitting Events from an
IO instance to an aggegator. This requires that each update take a
global telemetry lock, and is an impediment to richer telemetry.
This change removes the transport event types so that the Event and
Record types are left only to represent HTTP telemetry. Now, the
transport's IO type holds a reference to a shared `Metrics` structure.
As the transport is used, metric values are updated immediately.
A lock on the transport _registry_ is taken whenever a new transport is
opened/accepted and when metrics are reported. Each transport class's
metrics are now shared & locked independently, so it's possible for a
transport to update its metrics while the registry is being manipulated.
This has one functional change: the `tcp_read_bytes_total` and
`tcp_write_bytes_total` counters are now updated instantaneously.
Previously these values were only incremented on transport close, which
is misleading, especially for long-lived connections.
With this change, all transport-related telemetry logic lives in
`telemetry::transport`.
In anticipation of removing Transport-related Event types, we want to
separate the concerns of recording transport metrics updates from
reporting them to the metrics endpoint.
The transport module has been split into `Registry` and `Report` types:
`Registry` is responsible for recording updates, and `Report` is
responsible for rendering metrics.
Following #67 and #68, the `labels::TlsStatus` type can be removed in
favor of extending underlying `ctx::transport::TlsStatus` type to
implement `FmtLabels`.
There's no reason that we should ever need to cache proxy binaries
between CI runs.
For instance, for each pull request build we appear to cache a ~125M
binary.
Cache interactions seem to account for much of our CI times, so any
reduction here is helpful.
The `DstLabels` type has a large an undefined scope. Over time, it's
become critical in a variety of contexts, even outside of the scope of
prometheus endpoint labels.
This change makes `DstLabels` private to the
`telemetry::metrics::labels` module. This more clearly separates the
concerns of prometheus labeling from discovery, etc.
Now, destination metadata includes an index map of arbitrary metadata
labels.
Previously, dst label strings would be formatted eagerly at
service-discovery-time. This change moves this string formatting into
the data path(!). I think this is an acceptable tradeoff temporarily,
while we work to stop constructing RequestLabels in the data path
altogether.
There are many different types of things that implement `fmt::Display`
in the metrics library: lists of key-value labels, individial label keys
or values, groups of metrics output, etc. While modifying metrics code,
it can be very easy to use something that's not a label in the context
of a label.
In order to start dismantling the monolithic ctx structures, this change
removes the root `ctx::Process` type. This simplifies `ctx::Proxy` such
that is copyable and need not be `Arc`ed.
`telemetry::metrics::labels::Direction` has been updated to decorate
`ctx::Proxy` instead of modeling the same variants directly as an enum.
Previously, transport metrics have been stored in two "scopes": an
"open" scope, storing metrics by transport class, and a "close" scope
storing metrics by transport and end-of-stream classes.
Instead of storing these as parallel maps, this changes transport
metrics to be stored hierarchically so that the end-of-stream metrics
are stored _within_ the transport metrics type.
This will make it possible to share the transport metrics structure
directly with the transport wrapper so that a global lock need only be
taken at connect-time. Subsequent updates will then be scoped to the
shared transport structure.
This helps to setup linkerd/linkerd2#1430, among other improvements.
Furthermore, the `Scopes` type is no longer used for transport metrics,
since it's much easier to use the full affordances of `indexmap`,
especially now that it isn't part of an API.
The `telemetry::metrics::transport` module exposes much of its
implementation details to callers, which makes it difficult to make
changes to how the module is structured. In preparation for further
refactors, this change narrows the module's public API:
All labels and scopes types have been made private. A single, public
`Transports` type has been introduce to describe the entire public
interface of the module. This has been crafted to be free of `event`
types and to have minimal external dependencies.
A new `transport::Eos` type has been introduced to replace the
overloaded `labels::Classification` type -- this type was initially
introduced to model _HTTP response_ classification, but it was
reused for transports. This is an undesirable coupling that will have to
be broken when we start to adddress HTTP response classification
properly.
005d4f1 made `Metric::fmt_scopes` generic over an Iterator, but
maintained some of the unnecessary type bounds inherited from its
initial implementation.
This change relaxes the constraints on the label value so that it need
not be passed as a reference or be hashable--our only requirement is
that it can be rendered.
The `tcp_connect_err` tests do not pass on Linux.
I initially observed that the errno produced is ECONNREFUSED on Linux,
not EXFULL, but even changing this does not help the test to pass
reliably, as the socket teardown semantics appear to be slightly
different.
In order to prevent spurious test failures during development, this
change marks these two tests as macos-specific.
Various transport-specific labels are defined in the common
`metrics::labels` module.
In preparation to refactor transport metrics into Sensor and Report
halves, this change moves all transport-specific labels into the
transport metrics module.
Only the `DstLabels` and `Serve` types in `metrics` are used outside of
`telemetry`.
This change narrows visibility so that `metrics` is not referenced
outside of `telemetry`.
The `Scopes` type shouldn't be critical to metrics formatting.
This change makes the `Metric` type unaware of `Scopes` so that other
labeling mechanisms can be used if appropriate.
As we begin to remove event-based metrics telemetry, the `record` bench
test becomes an obstacle that needs to be updated at every step.
Given that we have already removed the event channel and plan to remove
the `Record` type entirely, it's suitable to remove the bench test
entirely.
While this is done, the `test-benches` Make target and rust-nightly
travis stage are obsoleted.
The metrics::Scopes type exposes its internal implementation to many of
its uses.
By extracting the type into its own module, we are forced to provide an
explicit public interface, hiding its IndexMap implementation details.
Following on #52, this change moves the
`telemetry::metrics::tls_config_reload` module to
`telemetry::tls_config_reload`.
The `Fmt` type has been renamed to `Report`.
Furthermore, two helper methods have been added to `metrics::Scopes` so
that its internal representation need not be made public.
As a follow-on to #50, which begins to tease apart metrics
infrastructure from application logic, this change moves the
`telemetry::metrics::process` module into `telemetry::process`.
The `Process` type has been renamed to `Report` and some other minor
cosmetic changes have been made.
The `telemetry::metrics` module has a mix of core metrics types and
application-specific code. I'd like to change this module to only
contain core types. All app-specific module will be moved into
`telemetry`.
In order to make these changes, the `metrics!` macro (and related types)
must be visible to to `telemetry`. This change moves the macro
definition into the parent mod.rs, and makes the `Metric`, `FmtMetric`,
and `Scopes` types public.
In subsequent PRs, app-specific logic will be moved from
`telemetry::metrics` to `telemetry`, and the `telemetry::metrics`
module's public API will be minimized.
Metric eviction was introduced to protect against the situation where,
over time, the proxy addresses an effectively unbounded number of
endpoints. Metrics with endpoint-specific dimensions must be removed
over time to prevent a form of memory leak.
Eviction is implemented for transport stats. However, transport stats do
not yet contain per-endpoint dimensions. The eviction logic is
superfluous in this case, since transport metrics are bounded and small.
In preparation of changes to transport telemetry, this disables tracking
of transport metrics for eviction. This logic will be restored when we
support per-endpoint transport metrics.
The `mk_err_enum` macro is only used in one place: the `labels::errno` module.
In order to make the main `labels` module simpler, the macro definition
can by moved into the errno module. If this macro is really generally
useful, it can be factored out later.
The current implementation of TLS config reload telemetry has several
layers: a sensor emits events into a dispatcher that updates metrics.
This can be simplified by sharing a common metrics object between the
metrics registry and config module.
The metrics registry is updated to hold a read-only `tls_config_reload::Fmt`;
and the config module holds a `tls_config_reload::Sensor`.
The `Sensor` type holds a strong reference to an inner metrics
structure and acquires a lock on updates.
The `Fmt` type holds a weak reference to the metrics structure so
that the metrics server can print the state as long as it's actually held
in memory. If the `Sensor` is dropped (for instance, because TLS has
been administratively disabled), then no metrics will be formatted by
`Fmt`.
Details of TLS configuration reload metrics span several modules.
This change consolidates all of this logic into a single
module with a single public type, `TlsConfigReload`, that supports
recording successful reloads, recording errors, and formatting its
current state.
This sets up further simplifications, eventually leading to the removal
of the `Event` API.
The `process` module exposes a `Sensor` type that is different from
other types called `Sensor`. Most `Sensor` types instrument other
types with telemetry. The `process::Sensor` type, on the other hand,
is used to read system metrics from the `/proc` filesystem, returning
a metrics summary.
Furthermore, `telemetry::metrics::Root` owns the process start time
metric.
In the interest of making the telemetry system more modular, this moves
all process-related telemetry concerns into the `process` module.
Instead of exposing a `Sensor` that produces metrics, a single public
`Process` type implements `fmt::Display` directly.
This removes process-related concerns from `telemetry/metrics/mod.rs` to
setup further refactoring along these lines.
This branch should not make any functional changes.
This branch makes two minor refactorings to the `client` module in
`control::destination::background`:
1. Remove the `AddOrigin` middleware and replace it with the
`tower-add-origin` crate from `tower-http`. These middlewares are
functionally identical, but the Tower version has tests.
2. Change `ClientService` from a type alias to a tuple struct. This
means that some of the middleware that are used only in this module
(`LogErrors` and `Backoff`) are no longer part of a publicly visible
type and can be made private to the module.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
When the destination service returns a hint that an endpoint is another
proxy, eligible HTTP1 requests are translated into HTTP2 and sent over
an HTTP2 connection. The original protocol details are encoded in a
header, `l5d-orig-proto`. When a proxy receives an inbound HTTP2
request with this header, the request is translated back into its HTTP/1
representation before being passed to the internal service.
Signed-off-by: Sean McArthur <sean@buoyant.io>
Oliver Gould
Sean McArthur
Sean McArthur
Eliza Weisman