Add tracing support to dispatcher and ingress #370

benmoss · 2021-07-27T16:58:25Z

🎁 Add tracing support to dispatcher and ingress

/kind enhancement

Release Note

Adds tracing with opencensus to the dispatcher and ingress

benmoss · 2021-07-27T16:59:56Z

An example trace:

[{"traceId":"5860bc60630c401309ddcaf28571e272","parentId":"c7470ed8613696ba","id":"9589a73243c3b651","kind":"SERVER","name":"cloudevents.http./","timestamp":1627404577247654,"duration":165,"localEndpoint":{"serviceName":"event-display-00003-deployment-768847c5d4-c9tqk","ipv4":"10.0.0.69"},"annotations":[{"timestamp":1627404577247669,"value":"Received 24 bytes"}],"tags":{"http.host":"event-display.default.svc.cluster.local","http.method":"POST","http.path":"/","http.status_code":"200","http.url":"/","http.user_agent":"Go-http-client/1.1","opencensus.status_description":"OK"}},{"traceId":"5860bc60630c401309ddcaf28571e272","parentId":"97d21f36f64cf47e","id":"ad37bc9d9d5a0e8e","kind":"SERVER","name":"/","timestamp":1627404577243057,"duration":272,"localEndpoint":{"serviceName":"default-broker-ingress-78566cd788-rzpqj","ipv4":"10.0.0.79"},"annotations":[{"timestamp":1627404577243076,"value":"Received 24 bytes"}],"tags":{"http.host":"default-broker-ingress.default.svc.cluster.local","http.method":"POST","http.path":"/","http.url":"/","http.user_agent":"Go-http-client/1.1"}},{"traceId":"5860bc60630c401309ddcaf28571e272","id":"e0d90f896186afdf","kind":"CLIENT","name":"cloudevents.client","timestamp":1627404577242216,"duration":1467,"localEndpoint":{"serviceName":"test-heartbeats-deployment-579c56b7cd-fdjzs","ipv4":"10.0.2.35"},"tags":{"cloudevents.datacontenttype":"application/json","cloudevents.id":"8783ac6b-c443-4e42-92df-184028604536","cloudevents.source":"https://knative.dev/eventing-contrib/cmd/heartbeats/#event-test/mypod","cloudevents.specversion":"1.0","cloudevents.type":"dev.knative.eventing.samples.heartbeat"}},{"traceId":"5860bc60630c401309ddcaf28571e272","parentId":"fa2dca30ea9d6d7e","id":"c7470ed8613696ba","kind":"CLIENT","name":"cloudevents.http.","timestamp":1627404577245956,"duration":2995,"localEndpoint":{"serviceName":"heartbeats-dispatcher-85bbd4bb66-25gp9","ipv4":"10.0.0.84"},"tags":{"http.host":"","http.method":"POST","http.path":"","http.status_code":"200","http.url":"http://event-display.default.svc.cluster.local","opencensus.status_description":"OK"}},{"traceId":"5860bc60630c401309ddcaf28571e272","parentId":"2d14868972054542","id":"fa2dca30ea9d6d7e","kind":"CLIENT","name":"cloudevents.client","timestamp":1627404577245756,"duration":3206,"localEndpoint":{"serviceName":"heartbeats-dispatcher-85bbd4bb66-25gp9","ipv4":"10.0.0.84"},"tags":{"cloudevents.datacontenttype":"application/json","cloudevents.id":"8783ac6b-c443-4e42-92df-184028604536","cloudevents.source":"https://knative.dev/eventing-contrib/cmd/heartbeats/#event-test/mypod","cloudevents.specversion":"1.0","cloudevents.type":"dev.knative.eventing.samples.heartbeat"}},{"traceId":"5860bc60630c401309ddcaf28571e272","parentId":"ad37bc9d9d5a0e8e","id":"2d14868972054542","kind":"SERVER","name":"rabbitmq_dispatcher","timestamp":1627404577244731,"duration":4728,"localEndpoint":{"serviceName":"heartbeats-dispatcher-85bbd4bb66-25gp9","ipv4":"10.0.0.84"},"tags":{"cloudevents.datacontenttype":"application/json","cloudevents.id":"8783ac6b-c443-4e42-92df-184028604536","cloudevents.source":"https://knative.dev/eventing-contrib/cmd/heartbeats/#event-test/mypod","cloudevents.specversion":"1.0","cloudevents.type":"dev.knative.eventing.samples.heartbeat"}}]

codecov · 2021-07-27T17:03:55Z

Codecov Report

Merging #370 (904a299) into main (73179e4) will decrease coverage by 0.53%.
The diff coverage is 83.00%.

❗ Current head 904a299 differs from pull request most recent head 46b3309. Consider uploading reports for the commit 46b3309 to get more accurate results

@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
- Coverage   75.97%   75.44%   -0.54%     
==========================================
  Files          47       47              
  Lines        2822     2879      +57     
==========================================
+ Hits         2144     2172      +28     
- Misses        551      569      +18     
- Partials      127      138      +11

Impacted Files	Coverage Δ
pkg/reconciler/trigger/resources/dispatcher.go	`92.22% <0.00%> (-2.10%)`	⬇️
pkg/reconciler/trigger/trigger.go	`60.37% <50.00%> (-0.08%)`	⬇️
pkg/reconciler/triggerstandalone/trigger.go	`59.57% <50.00%> (-0.09%)`	⬇️
pkg/dispatcher/dispatcher.go	`71.42% <68.62%> (-7.59%)`	⬇️
pkg/reconciler/broker/controller.go	`95.83% <75.00%> (-1.23%)`	⬇️
pkg/reconciler/brokerstandalone/controller.go	`94.82% <75.00%> (-1.47%)`	⬇️
pkg/reconciler/trigger/controller.go	`73.52% <75.00%> (+0.09%)`	⬆️
pkg/reconciler/triggerstandalone/controller.go	`64.70% <75.00%> (+0.87%)`	⬆️
pkg/reconciler/broker/resources/ingress.go	`97.22% <90.00%> (-2.78%)`	⬇️
...g/reconciler/brokerstandalone/resources/ingress.go	`97.22% <90.00%> (-2.78%)`	⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73179e4...46b3309. Read the comment docs.

benmoss · 2021-07-29T16:28:36Z

I think these aren't updating when the configmap changes, investigating further

/hold

benmoss · 2021-07-30T18:14:45Z

/hold cancel

vaikas

/hold
/lgtm
/approve

Just adding a hold since it's a release week. If you're fine with that, remove the hold, otherwise we can remove after the release. Up to you :)

pkg/dispatcher/dispatcher.go

pkg/reconciler/broker/controller.go

benmoss · 2021-08-09T13:32:46Z

Added a commit to fix up the string duplication / sprawl.

vaikas · 2021-08-10T08:33:42Z

Thanks for doing this!

Looks like you might have missed one place:

# knative.dev/eventing-rabbitmq/pkg/reconciler/brokerstandalone [knative.dev/eventing-rabbitmq/pkg/reconciler/brokerstandalone.test]
pkg/reconciler/brokerstandalone/controller.go:108: undefined: componentName

benmoss · 2021-08-10T15:10:44Z

that's a derp

benmoss · 2021-08-23T18:22:11Z

Investigating the test failures

samples/tracing/topology.yaml

samples/tracing/README.md

This is the version that RabbitMQ messaging-topology-operator v1.2.1 was tested against. Related to knative-extensions#370 (comment) Signed-off-by: Gerhard Lazu <glazu@vmware.com>

They will have K_TRACING_CONFIG, K_METRICS_CONFIG, and K_LOGGING_CONFIG set by the controller.

benmoss · 2021-12-13T17:14:20Z

Seem to have broken something with the latest rebase

/hold

gvmw · 2021-12-13T19:08:07Z

Having followed the Distributed tracing with Knative, OpenTelemetry and Jaeger steps on GKE 1.21, everything worked except the span in the heartbeats deployment which is not linked to anything:

This is what happened when I tested it with a RabbitMQ Broker:

ko apply -f config/broker
# ^^^ all ok 👍🏻

ko apply -f samples/tracing/
namespace/rmqbroker created
service.serving.knative.dev/subscriber created
containersource.sources.knative.dev/heartbeats created
trigger.eventing.knative.dev/default created
Error from server (NotFound): error when creating "STDIN": namespaces "rmqbroker" not found
Error from server (NotFound): error when creating "STDIN": namespaces "rmqbroker" not found
Error: error executing 'kubectl apply': exit status 1
2021/12/13 19:02:09 error during command execution:error executing 'kubectl apply': exit status 1

# 🤔 Can we add some numbers, e.g. 100-, 200-, etc.
# https://github.com/knative-sandbox/eventing-rabbitmq/tree/main/samples/multiple-namespaces
ko apply -f samples/tracing/
broker.eventing.knative.dev/default created
rabbitmqcluster.rabbitmq.com/cluster created
namespace/rmqbroker unchanged
service.serving.knative.dev/subscriber configured
containersource.sources.knative.dev/heartbeats configured
trigger.eventing.knative.dev/default unchanged

/lgtm
/approve

I am doing this optimistically & assuming that all tests will pass and @benmoss will add some numbers to those yaml files in samples/tracing

knative-prow-robot · 2021-12-13T19:08:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benmoss, gvmw, vaikas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [benmoss]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gvmw · 2021-12-13T19:25:13Z

/lgtm

benmoss · 2021-12-13T21:18:10Z

/hold cancel

gvmw · 2021-12-14T08:08:18Z

🚀

@benmoss

* Add Makefile for running complicated tasks with a single command For example, running end-to-end tests requires setting up dependencies, a local KiND cluster, installing operators, waiting for components to be ready, and then invoking `go test` with many flags. The entire flow is captured in .github/workflows/kind-e2e.yaml, so how do we run it locally? I think that there are some scripts which try to set everything up, but they are not discoverable, pretty sure they diverged from the CI workflow by now, and are not as composable as the Make equivalent. So how does it work? Type make and follow along: build Build binaries with e2e tags dep-update Update any dependency e2e-setup Setup environment for e2e testing env Configure shell env - eval "$(make env)" OR rm .env && make .env && source .env k9s Terminal ncurses UI for K8s test-e2e-broker Run Broker end-to-end tests test-e2e-publish Run TestKoPublish end-to-end tests test-e2e-source Run Source end-to-end tests test-unit Run unit tests To run end-to-end tests, which is what I had to do in the context of #492 (this PR is based on that one) I now only need to run the following: # This only needs to run once: make e2e-setup make test-e2e # I can run just a subset of tests with e.g.: make test-e2e-broker This iteration: - only support Linux & Darwin systems (does not support Windows) - only supports x84_64 (does not support latest Macs with ARM silicon) - defaults to the minimum supported K8s version (adding support for multiple K8s versions was out of scope) This was only tested on Linux Ubuntu 20.04.3 LTS, and should be tested on Darwin on Intel. Supporting Darwin on ARM should not be difficult, but as I don't have access to one, I cannot test. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Tell git to ingore .env and .config paths We do not want to version-control these Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Add KiND cluster config Used in `make kind-cluster` Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Install gh cli This comes in handy for checking out PRs, working with Actions, etc. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Add descriptions for install-* targets Allow versions to be changed just-in-time Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Bump k9s to latest Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Install kn CLI After reading the first 6 chapters of Knative in Action, it became obvious to me that this CLI is essential for working with Knative. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Move KO_URL after KO_BIN_DIR On Windows on WSL with make 4.3 this fails. Not sure why it worked on Linux with make 4.2.1 as it's clearly wrong. This fixes it. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Use make variables that expand only once Variables declared with := get interpreted a single time while those with = get evaluated every time they are invoked. We want just once invocation. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Install serving part of the e2e setup This is the easiest way to setup Sinks (they only need to be addressable). Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Install Kourier for ingress part of Knative Serving Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Respect KUBECONFIG from env & do not hard-code to kind Enables running against any K8s, helpful when testing against GKE. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Bump cert-manager to 1.5.4 This is the version that RabbitMQ messaging-topology-operator v1.2.1 was tested against. Related to #370 (comment) Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Fix missing OPEN Great spot @benmoss! Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Make test-e2e depend on KUBECONFIG, no more roundabouts @benmoss made an excellent point in this comment, and I think that this change address it. https://github.com/knative-sandbox/eventing-rabbitmq/pull/525/files#r759457509 Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Remove kind setup scripts These are now replaced by the Makefile, for context see #525 Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Use the existing context for the default install Great suggestion @benmoss! Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Make install & test-e2e behaviour is more intuitive install takes care of everything, and test-e2e also runs it - they are self-contained targets. The other test-e2e-* targets make assumptions that an install has run before. The idea is that running a subset of e2e tests should be as fast as possible, while running the entire e2e should be complete. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Make the default help target more helpful for newcomers That .env is always a gotcha... direnv doesn't make it better. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Rename build to test-compilation I like it @benmoss! https://github.com/knative-sandbox/eventing-rabbitmq/pull/525/files#r765968362 Signed-off-by: Gerhard Lazu <glazu@vmware.com> * List test-unit-uncached in help This is something that @ikvmw was mentioning yesterday. Making this option visibile should help with WTF moments that originate in a stale cache. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Use Makefile in kind conformance standalone GitHub Action The CI will continuously validate that everything is wired up correctly, and devs using this locally will have the confidence that it just works. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Fix envsubst variables in kind.yaml Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Do not link ko to gcloud This is only necessary if pushing images to gcr.io, which is what *some* of us do locally, but not everyone. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Uppercase all instances of KiND & K8s Otherwise weird things happen, like KiND_CLUSTER_NAME vs KIND_CLUSTER_NAME. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Export updated PATH so that all targets work correctly Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Bump gh, k9s, kn & Knative versions to latest Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Use make targets in kind e2e test GitHub Action Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Install standalone broker for standalone conformance tests Thanks @benmoss for spotting it! #525 (comment) Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Separate standalone conformance tests from the regular ones Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Make test-conformance targets leaner So that they are less likely to fail in GitHub Actions. Signed-off-by: Gerhard Lazu <glazu@vmware.com> * Convert standard conformance tests to use make targets Signed-off-by: Gerhard Lazu <glazu@vmware.com>

knative-prow-robot added the kind/enhancement label Jul 27, 2021

google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 27, 2021

knative-prow-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 27, 2021

benmoss force-pushed the tracing branch 2 times, most recently from de8e3f8 to d7baff7 Compare July 28, 2021 14:50

benmoss closed this Jul 28, 2021

benmoss reopened this Jul 28, 2021

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2021

benmoss force-pushed the tracing branch from d7baff7 to 57d1f3f Compare July 30, 2021 18:12

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 30, 2021

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 9, 2021

vaikas reviewed Aug 9, 2021

View reviewed changes

pkg/dispatcher/dispatcher.go Outdated Show resolved Hide resolved

pkg/reconciler/broker/controller.go Outdated Show resolved Hide resolved

knative-prow-robot assigned vaikas Aug 9, 2021

knative-prow-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed lgtm Indicates that a PR is ready to be merged. labels Aug 9, 2021

benmoss force-pushed the tracing branch from 58250f4 to 736bbb8 Compare August 10, 2021 15:11

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2021

benmoss force-pushed the tracing branch from 736bbb8 to 9d2680c Compare August 19, 2021 16:19

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2021

benmoss force-pushed the tracing branch from 9d2680c to ba070e0 Compare August 23, 2021 12:46

benmoss force-pushed the tracing branch from ba070e0 to 4c6a98d Compare August 25, 2021 19:46

gvmw reviewed Dec 6, 2021

View reviewed changes

samples/tracing/topology.yaml Outdated Show resolved Hide resolved

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2021

gvmw reviewed Dec 6, 2021

View reviewed changes

samples/tracing/README.md Show resolved Hide resolved

benmoss force-pushed the tracing branch from c07fdab to 5c36163 Compare December 9, 2021 16:29

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 9, 2021

Add tracing support to dispatcher and ingress

71304db

They will have K_TRACING_CONFIG, K_METRICS_CONFIG, and K_LOGGING_CONFIG set by the controller.

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 13, 2021

benmoss force-pushed the tracing branch from 7f8e23a to db28694 Compare December 13, 2021 15:20

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 13, 2021

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 13, 2021

Ben Moss added 3 commits December 13, 2021 14:02

Refactor dispatcher to allow deferring span.End

3a9dd71

Add tracing sample

6850d0c

Split sample into many files

904a299

benmoss force-pushed the tracing branch from db28694 to 904a299 Compare December 13, 2021 19:02

knative-prow-robot assigned gvmw Dec 13, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2021

ensure sample ns gets created first

46b3309

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2021

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 13, 2021

knative-prow-robot merged commit e4db575 into knative-extensions:main Dec 13, 2021

benmoss deleted the tracing branch December 13, 2021 21:34

Add tracing support to dispatcher and ingress #370

Add tracing support to dispatcher and ingress #370

Uh oh!

Conversation

benmoss commented Jul 27, 2021

Uh oh!

benmoss commented Jul 27, 2021

Uh oh!

codecov bot commented Jul 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

benmoss commented Jul 29, 2021

Uh oh!

benmoss commented Jul 30, 2021

Uh oh!

vaikas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benmoss commented Aug 9, 2021

Uh oh!

vaikas commented Aug 10, 2021

Uh oh!

benmoss commented Aug 10, 2021

Uh oh!

benmoss commented Aug 23, 2021

Uh oh!

Uh oh!

Uh oh!

benmoss commented Dec 13, 2021

Uh oh!

gvmw commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knative-prow-robot commented Dec 13, 2021

Uh oh!

gvmw commented Dec 13, 2021

Uh oh!

benmoss commented Dec 13, 2021

Uh oh!

gvmw commented Dec 14, 2021

Uh oh!

Uh oh!

codecov bot commented Jul 27, 2021 •

edited

Loading

gvmw commented Dec 13, 2021 •

edited

Loading