feat: Create base config for observability PoC #80
Conversation
Codecov ReportBase: 43.18% // Head: 43.56% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #80 +/- ##
==========================================
+ Coverage 43.18% 43.56% +0.38%
==========================================
Files 45 44 -1
Lines 1936 1919 -17
==========================================
Hits 836 836
+ Misses 1100 1083 -17
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
foo0x29a
requested review from
Freyskeyd,
atanmarko and
sebastiendan
and removed request for
a team and
atanmarko
| .with_exporter( | ||
| opentelemetry_otlp::new_exporter() | ||
| .tonic() | ||
| .with_endpoint("http://otel-collector:4317"), |
There was a problem hiding this comment.
We would need to keep using a cli arg or an env variable (similarly to jaeger_agent).
crates/topos-tce/src/main.rs
Outdated
| @@ -47,6 +54,25 @@ async fn main() { | |||
| .install_batch(opentelemetry::runtime::Tokio) | |||
| .unwrap(); | |||
|
|
|||
| let export_config = ExportConfig { | |||
| endpoint: "http://otel-collector:4317".to_string(), | |||
There was a problem hiding this comment.
We would need to keep using a cli arg or an env variable (similarly to jaeger_agent).
There was a problem hiding this comment.
Yes definitely. Specially for the cases where users would like to opt-out and do not send any telemetry data.
I don't have confidence in Rust, so kept the code minimal.
| let tracer = opentelemetry_otlp::new_pipeline() | ||
| .tracing() | ||
| .with_exporter( | ||
| opentelemetry_otlp::new_exporter() |
There was a problem hiding this comment.
We're using the same exporter builder, maybe we can mutualise the code here
There was a problem hiding this comment.
@Freyskeyd, thanks for reviewing this PR!
Do you mind taking over the instrumentation bit? It would be great if you pushed some changes to this branch fixing the already existing minimal setup!
I'm probably not the most appropriate person to touch the code base. I made some changes on the main.rs and Cargo.toml files, but am not entirely sure if that's the optimal solution.
| volumes: | ||
| - ./otel-collector-config.yaml:/etc/otel-collector-config.yaml | ||
| ports: | ||
| - "1888:1888" # pprof extension |
There was a problem hiding this comment.
Most ports don't need host bindings, no? Actually this is a thought I've had for a while: With docker-compose, having links created automatically between all containers on a common network, we don't event need to specify any port (neither host nor container), no?
There was a problem hiding this comment.
Most ports don't need host bindings, no?
Yes you're correct. Will leave only the necessary for the PoC, thanks for spotting this!
With docker-compose, having links created automatically between all containers on a common network, we don't event need to specify any port (neither host nor container), no?
That's correct. The difference, however, is if we define the ports field but omit the ${HOST} value, docker compose will assign a random host port:
$ docker compose ps
NAME COMMAND SERVICE STATUS PORTS
boot "/bin/sh -c ./topos-…" boot running 0.0.0.0:57131->9090/tcp
jaeger "/go/bin/all-in-one-…" jaeger running 5775/udp, 5778/tcp, 0.0.0.0:14250->14250/tcp, 6831-6832/udp, 14268/tcp, 0.0.0.0:16685-16686->16685-16686/tcp
prometheus "/bin/prometheus --c…" prometheus running 0.0.0.0:9090->9090/tcp
spam "./cert-spammer" cert-spammer running
tools-otel-collector-1 "/otelcontribcol --c…" otel-collector running 0.0.0.0:1888->1888/tcp, 0.0.0.0:4317->4317/tcp, 0.0.0.0:8888-8889->8888-8889/tcp, 0.0.0.0:13133->13133/tcp, 0.0.0.0:55679->55679/tcp, 55680/tcp
tools-peer-1 "/bin/sh -c ./topos-…" peer running 0.0.0.0:57140->1340/tcp, 0.0.0.0:57141->9090/tcp
tools-peer-2 "/bin/sh -c ./topos-…" peer running 0.0.0.0:57132->1340/tcp, 0.0.0.0:57133->9090/tcp
tools-peer-3 "/bin/sh -c ./topos-…" peer running 0.0.0.0:57136->1340/tcp, 0.0.0.0:57137->9090/tcp
tools-peer-4 "/bin/sh -c ./topos-…" peer running 0.0.0.0:57138->1340/tcp, 0.0.0.0:57139->9090/tcp
tools-peer-5 "/bin/sh -c ./topos-…" peer running 0.0.0.0:57134->1340/tcp, 0.0.0.0:57135->9090/tcp
There was a problem hiding this comment.
The difference, however, is if we define the ports field but omit the ${HOST} value, docker compose will assign a random host port
Yep, but what I meant is that I think we tend to use random port assignation "by default" because we think that ports should be specified. Let me describe an example:
version: '3.5'
services:
one:
image: ...
ports:
- 3000
two:
image: ...
env:
- "endpoint=http://one:3000"Here often we would want to specify the 3000 port on one because we want two to access it, but I believe this isn't needed. (this was my comment above)
|
Closing as #109 was merged. |
TL;DR: Add base config for TCE Observability, including:
Before
Observability is a broad topic that essentially covers at least 3 areas:
The design of our current Observability stack is as follows:
After
There are evidences that OpenTelemetry has been gaining some traction in the Observability space. We have recently started the discussion of possibly adopting the OpenTelemetry collection of tools as follows:
In this new design, we add one more layer - OpenTelemetry Collector, to allow standardized and vendor-agnostic telemetry data.
ATTENTION: THIS IS EXPERIMENTAL. WE ARE OPEN FOR CHANGES.
OpenTelemetry
Configuration
The
otel-collector-config.yamlfile was inspired by this official demo. The complete reference can be found here.As receivers we define:
And as exporters:
Instrumentation
The instrumentation is done using the opentelemetry-otlp crate.
How to run
From the root directory of TCE, run:
Jaeger UI will be available at http://localhost:16686/ :
And Prometheus at http://localhost:9090/:
After work is finished:
Caveats:
$GITHUB_TOKENis defined and contains a GitHub personal token with appropriate access to build the project