Add Open Telemetry Spec attributes to IMC Dispatcher span by xtreme-sameer-vohra · Pull Request #4659 · knative/eventing

xtreme-sameer-vohra · 2020-12-15T20:53:58Z

Proposed Changes

imc dispatcher adds the following attributes to trace span to make it consistent with broker ingress and filter spans

      messaging.destination
      messaging.message_id
      messaging.protocol
      messaging.system

update channel spec observability section to reference the Open Telemetry Spec attributes
update channel spec for readability
🧹 Update or clean up current behavior

Release Note

The  `imc-dispatcher` service adds new trace span attributes to be consistent with `broker-ingress.knative-eventing` & `broker-filter.knative-eventing` services. The new attributes are messaging.destination,  messaging.message_id, messaging.protocol and messaging.system

xtreme-sameer-vohra · 2020-12-15T20:55:41Z

I wasn't sure if there is a better approach to populate the destination attribute so the simple approach uses url, which is accurate but not as user friendly as those for the other 2 services.

codecov · 2020-12-15T21:02:00Z

Codecov Report

Merging #4659 (ecb6684) into master (3aff77d) will increase coverage by 0.00%.
The diff coverage is 72.72%.

@@           Coverage Diff           @@
##           master    #4659   +/-   ##
=======================================
  Coverage   81.07%   81.08%           
=======================================
  Files         291      291           
  Lines        8216     8220    +4     
=======================================
+ Hits         6661     6665    +4     
  Misses       1154     1154           
  Partials      401      401

Impacted Files	Coverage Δ
pkg/channel/message_dispatcher.go	`77.31% <0.00%> (ø)`
pkg/tracing/populate_span_transformer.go	`73.33% <80.00%> (ø)`

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 3aff77d...9ed38da. Read the comment docs.

matzew · 2020-12-16T07:14:58Z

@skonto @pierDipi mind to look ?

slinkydeveloper · 2020-12-16T09:01:16Z

+The Channel MUST recognize and pass through all tracing information from sender
+to subscribers using [W3C Tracecontext](https://w3c.github.io/trace-context/),
+although internally it MAY use another mechanism(s) to propagate the tracing
+information. The Channel SHOULD sample and write traces to the location
+specified in
+[`config-tracing`](https://github.com/knative/eventing/blob/master/config/config-tracing.yaml).


Same applies for broker

It seems to me the observabiliity parts of broker and channel should be exactly the same. I am wondering if it worths it is own spec and just reference the spec in both channel and broker. Adam also mentioned that whether the broker should have some common metrics, but I remembered the last conversation ended with not going to implement it.

@zhongduo thats a fair point.
In the current spec, the channel has an additional requirement of passing through tracing info from sender to subscribers. It sounds like that would be applicable for the Broker as well ?

It sounds like that would be applicable for the Broker as well ?

Yep, and similar to channel, there is no constraint on how to propagate tracing informations

I think it will be very confusing if there are difference between Channel and broker, as you can build a "broker" using channels.

as you can build a "broker" using channels.

But that would be an implementation detail of how you build that particular broker

I wasn't really thinking about "how to build the broker". My point is that in tracing side, most requirements in broker will apply in channel as well, so just wondering how we can avoid such an inconsistency. I do understand theoretically they can be very different, but at least now they looks related to me.

zhongduo · 2020-12-16T14:41:35Z

 - the backoff rate

-#### Metrics
+#### Observability


This name change makes perfect sense. Thanks for catching that.

zhongduo · 2020-12-16T14:45:51Z

+The Channel MUST recognize and pass through all tracing information from sender
+to subscribers using [W3C Tracecontext](https://w3c.github.io/trace-context/),
+although internally it MAY use another mechanism(s) to propagate the tracing
+information. The Channel SHOULD sample and write traces to the location
+specified in
+[`config-tracing`](https://github.com/knative/eventing/blob/master/config/config-tracing.yaml).


It seems to me the observabiliity parts of broker and channel should be exactly the same. I am wondering if it worths it is own spec and just reference the spec in both channel and broker. Adam also mentioned that whether the broker should have some common metrics, but I remembered the last conversation ended with not going to implement it.

zhongduo · 2020-12-16T16:25:30Z

I am not talking about the implementation. I guess my point is that in some sense they are originally designed to be mostly equivalent. So keep specs, including observability, synced should be the default, unless we have a reason not to.

…

On Wed, Dec 16, 2020 at 11:21 AM Francesco Guardiani < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/spec/channel.md <#4659 (comment)>: > +The Channel MUST recognize and pass through all tracing information from sender +to subscribers using [W3C Tracecontext](https://w3c.github.io/trace-context/), +although internally it MAY use another mechanism(s) to propagate the tracing +information. The Channel SHOULD sample and write traces to the location +specified in +[`config-tracing`](https://github.com/knative/eventing/blob/master/config/config-tracing.yaml). as you can build a "broker" using channels. But that would be an implementation detail of how you build that particular broker — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4659 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACE6CNANVESZF5QFM3MY3DLSVDNA7ANCNFSM4U46QO3Q> .

xtreme-sameer-vohra · 2020-12-18T22:25:48Z

/retest

xtreme-sameer-vohra · 2020-12-18T23:08:37Z

I was able to run the rekt -> TestBrokerAsMiddleware locally

go test -v -tags=e2e -count=1 ./test/rekt -run ^TestBrokerAsMiddleware$
2020/12/18 17:04:17 PWD:  /Users/svohra/go/src/knative.dev/eventing/test/rekt
2020/12/18 17:04:17 FILENAME:  /Users/svohra/go/src/knative.dev/eventing/vendor/knative.dev/reconciler-test/pkg/eventshub/resources.go
{"level":"info","ts":1608332657.09967,"logger":"fallback","caller":"injection/injection.go:61","msg":"Starting informers..."}
level All, state Any

=== RUN   TestBrokerAsMiddleware
=== PAUSE TestBrokerAsMiddleware
=== CONT  TestBrokerAsMiddleware
ko publish -B knative.dev/reconciler-test/cmd/eventshub
=== RUN   TestBrokerAsMiddleware/Prerequisite
=== RUN   TestBrokerAsMiddleware/Prerequisite/Setup
=== RUN   TestBrokerAsMiddleware/Prerequisite/Setup/install_broker_"default"
2020/12/18 17:04:39 PWD:  /Users/svohra/go/src/knative.dev/eventing/test/rekt
2020/12/18 17:04:39 FILENAME:  /Users/svohra/go/src/knative.dev/eventing/test/rekt/resources/broker/broker.go
2020/12/18 17:04:39 new files in /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml751265804
I1218 17:04:39.280437   46057 manifest.go:62] Reading YAML filepath: /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml751265804 recursive: false
I1218 17:04:39.934307   46057 manifest.go:85] Creating type eventing.knative.dev/v1, Kind=Broker name default
2020/12/18 17:04:40 Created:
2020/12/18 17:04:40 {Broker rekt-uiuxjrcp default  eventing.knative.dev/v1  }
=== RUN   TestBrokerAsMiddleware/Prerequisite/Requirement
=== RUN   TestBrokerAsMiddleware/Prerequisite/Assert
=== RUN   TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_ready
=== PAUSE TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_ready
=== RUN   TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_addressable
=== PAUSE TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_addressable
=== CONT  TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_ready
=== CONT  TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_addressable
2020/12/18 17:04:40 eventing.knative.dev/v1, Resource=brokers default has no status.address.url, %!s(<nil>)
2020/12/18 17:04:43 eventing.knative.dev/v1, Resource=brokers default is addressable: http://broker-ingress.knative-eventing.svc.cluster.local/rekt-uiuxjrcp/default
=== RUN   TestBrokerAsMiddleware/Prerequisite/Teardown
=== RUN   TestBrokerAsMiddleware/Setup
=== RUN   TestBrokerAsMiddleware/Setup/install_sink_and_install_trigger_and_trigger_goes_ready_and_install_source
2020/12/18 17:04:43 PWD:  /Users/svohra/go/src/knative.dev/eventing/test/rekt
2020/12/18 17:04:43 FILENAME:  /Users/svohra/go/src/knative.dev/eventing/vendor/knative.dev/reconciler-test/pkg/eventshub/resources.go
2020/12/18 17:04:43 new files in /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml160034590
I1218 17:04:43.626228   46057 manifest.go:62] Reading YAML filepath: /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml160034590 recursive: false
I1218 17:04:43.705574   46057 manifest.go:85] Creating type /v1, Kind=ServiceAccount name sink-ackvblvf
I1218 17:04:43.865522   46057 manifest.go:85] Creating type rbac.authorization.k8s.io/v1, Kind=Role name sink-ackvblvf
I1218 17:04:44.075676   46057 manifest.go:85] Creating type rbac.authorization.k8s.io/v1, Kind=RoleBinding name sink-ackvblvf
I1218 17:04:44.257005   46057 manifest.go:85] Creating type /v1, Kind=Service name sink-ackvblvf
I1218 17:04:44.386232   46057 manifest.go:85] Creating type /v1, Kind=Pod name sink-ackvblvf
2020/12/18 17:04:44 Created:
2020/12/18 17:04:44 {ServiceAccount rekt-uiuxjrcp sink-ackvblvf  v1  }
2020/12/18 17:04:44 {Role rekt-uiuxjrcp sink-ackvblvf  rbac.authorization.k8s.io/v1  }
2020/12/18 17:04:44 {RoleBinding rekt-uiuxjrcp sink-ackvblvf  rbac.authorization.k8s.io/v1  }
2020/12/18 17:04:44 {Service rekt-uiuxjrcp sink-ackvblvf  v1  }
2020/12/18 17:04:44 {Pod rekt-uiuxjrcp sink-ackvblvf  v1  }
    event_info_store.go:73: Store added to the EventListener, which has already seen 5 events
2020/12/18 17:04:50 PWD:  /Users/svohra/go/src/knative.dev/eventing/test/rekt
2020/12/18 17:04:50 FILENAME:  /Users/svohra/go/src/knative.dev/eventing/test/rekt/resources/trigger/trigger.go
2020/12/18 17:04:50 new files in /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml989034217
I1218 17:04:50.831243   46057 manifest.go:62] Reading YAML filepath: /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml989034217 recursive: false
I1218 17:04:50.890595   46057 manifest.go:85] Creating type eventing.knative.dev/v1, Kind=Trigger name via-ttbbsqle
2020/12/18 17:04:50 Created:
2020/12/18 17:04:50 {Trigger rekt-uiuxjrcp via-ttbbsqle  eventing.knative.dev/v1  }
2020/12/18 17:04:51 PWD:  /Users/svohra/go/src/knative.dev/eventing/test/rekt
2020/12/18 17:04:51 FILENAME:  /Users/svohra/go/src/knative.dev/eventing/vendor/knative.dev/reconciler-test/pkg/eventshub/resources.go
2020/12/18 17:04:51 new files in /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml730321155
I1218 17:04:51.020240   46057 manifest.go:62] Reading YAML filepath: /var/folders/mf/9ld4g_fn6kz01xylk1p65vgm0000gp/T/processed_yaml730321155 recursive: false
I1218 17:04:51.086970   46057 manifest.go:85] Creating type /v1, Kind=ServiceAccount name source-ipvbprvm
I1218 17:04:51.176327   46057 manifest.go:85] Creating type rbac.authorization.k8s.io/v1, Kind=Role name source-ipvbprvm
I1218 17:04:51.276325   46057 manifest.go:85] Creating type rbac.authorization.k8s.io/v1, Kind=RoleBinding name source-ipvbprvm
I1218 17:04:51.492733   46057 manifest.go:85] Creating type /v1, Kind=Service name source-ipvbprvm
I1218 17:04:51.613690   46057 manifest.go:85] Creating type /v1, Kind=Pod name source-ipvbprvm
2020/12/18 17:04:51 Created:
2020/12/18 17:04:51 {ServiceAccount rekt-uiuxjrcp source-ipvbprvm  v1  }
2020/12/18 17:04:51 {Role rekt-uiuxjrcp source-ipvbprvm  rbac.authorization.k8s.io/v1  }
2020/12/18 17:04:51 {RoleBinding rekt-uiuxjrcp source-ipvbprvm  rbac.authorization.k8s.io/v1  }
2020/12/18 17:04:51 {Service rekt-uiuxjrcp source-ipvbprvm  v1  }
2020/12/18 17:04:51 {Pod rekt-uiuxjrcp source-ipvbprvm  v1  }
    event_info_store.go:73: Store added to the EventListener, which has already seen 13 events
=== RUN   TestBrokerAsMiddleware/Requirement
=== RUN   TestBrokerAsMiddleware/Assert
=== RUN   TestBrokerAsMiddleware/Assert/[Stable/MUST]broker_as_middleware_deliver_an_event
=== PAUSE TestBrokerAsMiddleware/Assert/[Stable/MUST]broker_as_middleware_deliver_an_event
=== CONT  TestBrokerAsMiddleware/Assert/[Stable/MUST]broker_as_middleware_deliver_an_event
=== RUN   TestBrokerAsMiddleware/Teardown
--- PASS: TestBrokerAsMiddleware (43.77s)
    --- PASS: TestBrokerAsMiddleware/Prerequisite (4.35s)
        --- PASS: TestBrokerAsMiddleware/Prerequisite/Setup (1.16s)
            --- PASS: TestBrokerAsMiddleware/Prerequisite/Setup/install_broker_"default" (1.16s)
        --- PASS: TestBrokerAsMiddleware/Prerequisite/Requirement (0.00s)
        --- PASS: TestBrokerAsMiddleware/Prerequisite/Assert (0.00s)
            --- PASS: TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_ready (0.03s)
            --- PASS: TestBrokerAsMiddleware/Prerequisite/Assert/[Stable/MUST]broker_be_addressable (3.19s)
        --- PASS: TestBrokerAsMiddleware/Prerequisite/Teardown (0.00s)
    --- PASS: TestBrokerAsMiddleware/Setup (13.13s)
        --- PASS: TestBrokerAsMiddleware/Setup/install_sink_and_install_trigger_and_trigger_goes_ready_and_install_source (13.13s)
    --- PASS: TestBrokerAsMiddleware/Requirement (0.00s)
    event_info_store.go:161: Assert passed
    event_info_store.go:173: Assert passed
    event_info_store.go:200: Assert passed
    --- PASS: TestBrokerAsMiddleware/Assert (0.00s)
        --- PASS: TestBrokerAsMiddleware/Assert/[Stable/MUST]broker_as_middleware_deliver_an_event (4.01s)
    --- PASS: TestBrokerAsMiddleware/Teardown (0.00s)
PASS
ok  	knative.dev/eventing/test/rekt	43.914s

Am I missing something or is the test a flake ?

slinkydeveloper · 2020-12-21T09:31:17Z

+	// 3. Channel Dispatcher span
+	// 4. Channel sends event to Mutator pod.


Why 2 spans here? Are we sure this isn't an impl detail here and only 1 match should be done (the point 3)?

I discovered the the span containing the cloudevent & tracing tags wasn't being tested previously.

Removing the assertion for the span corresponding to Channel sends event to Mutator pod doesn't impact the issue this PR is attempting to address. However, It would just loose the test case to ensure the http status of the resultant request is not tested. I'd defer the importance of testing/not testing to folks with more context.

Looking at the broker tests, it doesn't appear that they are asserting for the presence of the span with the http status either.

So, I am happy to drop the assertions for spans corresponding to 4, 7 and 10.

Does that make sense @slinkydeveloper ?

Sorry, I think i explained myself wrongly. Why do assert the span:

Channel Dispatcher span

Isn't that an implementation detail?

Thats the span that contains the Open Telemetry Spec Attributes that this PR adds
Ditto for spans labeled 6 and 9

slinkydeveloper · 2020-12-21T09:32:44Z


 	if span.IsRecordingEvents() {
-		err = kncloudevents.WriteHTTPRequestWithAdditionalHeaders(ctx, message, req, additionalHeaders, kncloudevents.PopulateSpan(span))
+		err = kncloudevents.WriteHTTPRequestWithAdditionalHeaders(ctx, message, req, additionalHeaders, tracing.PopulateSpan(span, url.String()))


Since you're keeping kncloudevents.PopulateSpan (which is fine, to avoid breaking downstream), can you add the deprecated comment to such function to signal users to use this new one in tracing module?

The kncloudevents.PopulateSpan function & file was removed in the PR. However, I hadn't considered how this change might impact downstream.
The only references to PopulateSpan were from message_dispatcher.go, which I assumed would be shipped together. Are there downstream uses cases that would be affected ?

I honestly don't remember, try to remove the function and check if the downstream checks break 😄

Since this PR removes the function I was looking at the downstream checks for the PR. It looks like all but the Kafka downstream checks are passing. I am not sure if the Kafka failure is related to the changes in the PR.

I also did a github search in knative org and knative-sandbox org and the only reference is message_dispatcher.go, so this change feels safe.

Any additional things I could check ?

Hmm, rebased off master just now and now the downstream kafak test is passing but the rabbit and nats ones are failing due to go mod issues.

skonto · 2020-12-21T10:49:27Z

+
 	expectedAttributes := map[string]interface{}{
+		"messaging.system":        "knative",
+		"messaging.protocol":      "HTTP",


I see that http is the protocol used, do we support https? How about protocol version maybe we should be explicit as there is wip (if I understood correctly) for http/2?

Hey @skonto
If I understood correctly, once the http/2 feature is wrapped up, the protocol string could change and hence hardcoding a value in the test is not a good idea.

The current implementation has it hardcoded here which would need to change once the http/2 feature is completed.

If that change is imminent;

the assertion could be updated to match a regex

the string could be pulled out and put into an enum ( would still only be configured for HTTP for now )

and if the test assertion for protocol isn't very valuable, the assertion for protocol could be dropped

wdyt ?

slinkydeveloper

Minor things, but for me we're good to go

slinkydeveloper · 2021-01-07T08:44:11Z


 	if span.IsRecordingEvents() {
-		err = kncloudevents.WriteHTTPRequestWithAdditionalHeaders(ctx, message, req, additionalHeaders, kncloudevents.PopulateSpan(span))
+		err = kncloudevents.WriteHTTPRequestWithAdditionalHeaders(ctx, message, req, additionalHeaders, tracing.PopulateSpan(span, url.String()))


I honestly don't remember, try to remove the function and check if the downstream checks break 😄

slinkydeveloper · 2021-01-07T08:49:18Z

+	// 3. Channel Dispatcher span
+	// 4. Channel sends event to Mutator pod.


Sorry, I think i explained myself wrongly. Why do assert the span:

Channel Dispatcher span

Isn't that an implementation detail?

- trace should include Open Telemetry Spec attributes (messaging.*) messaging.destination messaging.message_id messaging.protocol messaging.system

- change Metrics -> Observability, to match spec/broker.md - relocate trace pass through & trace config information from Misc to Observability sections - add section specifying messaging.* attributes that should be included in the trace span Signed-off-by: Sameer Vohra <vsameer@vmware.com>

- add check for Open Telemetry Spec tags (messaging.*) in spans emitted by message dispatcher - exported helpful regex host suffix - re-worked the channel trace tree to include all the missing dispatcher spans. Signed-off-by: Sameer Vohra <vsameer@vmware.com>

- nit: change transformer -> mutator for consistency Signed-off-by: Sameer Vohra <vsameer@vmware.com>

knative-metrics-robot · 2021-01-07T15:52:38Z

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/tracing/populate_span_transformer.go	Do not exist	86.7%

xtreme-sameer-vohra · 2021-01-12T16:54:16Z

Minor things, but for me we're good to go

Hey @slinkydeveloper
I responded in the threads for each of those. Could I get your take on them and if you feel more changes are required to merge the change ?

slinkydeveloper · 2021-01-12T17:26:09Z

For me this is good to go and I see all comments were addressed
/approve
/lgtm

vaikas · 2021-01-14T11:52:00Z

Conformance / upgrade failed to bring the cluster up:

targetLink: 'https://container.googleapis.com/v1beta1/projects/915326016979/locations/us-central1/clusters/e2e-cls-dxbpqzgi'
 zone: 'us-central1'>] finished with error: All cluster resources were brought up, but: component "kube-apiserver" from endpoint "gke-40ccdc8f54a64a9a96a5-71bc" is unhealthy.
W0112 18:17:50.232314     279 up.go:44] repo-root not supplied, skip dumping cluster logs
Error: error creating clusters: error creating cluster: exit status 1
2021/01/12 18:17:50 Failed to run tests with kubetest2: exit status 1

Broker middlewater is #4637
/retest

vaikas · 2021-01-14T11:54:34Z

Also rerunning the downstream tests, natss/rabbitmq should be fixed, but keda is being worked on.

vaikas · 2021-01-14T12:34:32Z

As expected keda is still failing, everything else is passing.

Thanks for doing this @xtreme-sameer-vohra
/lgtm
/approve

knative-prow-robot · 2021-01-14T12:34:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: slinkydeveloper, vaikas, xtreme-sameer-vohra

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [slinkydeveloper,vaikas]

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

google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Dec 15, 2020

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 15, 2020

knative-prow-robot requested review from slinkydeveloper and tayarani December 15, 2020 20:54

xtreme-sameer-vohra force-pushed the 4578-trace-attr branch 2 times, most recently from 27dfb0b to 8f47744 Compare December 15, 2020 22:56

matzew reviewed Dec 16, 2020

View reviewed changes

Comment thread docs/spec/channel.md

slinkydeveloper reviewed Dec 16, 2020

View reviewed changes

pierDipi reviewed Dec 16, 2020

View reviewed changes

Comment thread docs/spec/channel.md Outdated

Comment thread docs/spec/channel.md Outdated

zhongduo reviewed Dec 16, 2020

View reviewed changes

xtreme-sameer-vohra force-pushed the 4578-trace-attr branch from 8f47744 to 1e6fa2e Compare December 16, 2020 15:44

xtreme-sameer-vohra force-pushed the 4578-trace-attr branch from 1e6fa2e to 26c3de1 Compare December 16, 2020 21:10

xtreme-sameer-vohra changed the title ~~Add Open Telemetry Spec attributes to IMC Dispatcher span~~ WIP: Add Open Telemetry Spec attributes to IMC Dispatcher span Dec 16, 2020

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 16, 2020

slinkydeveloper reviewed Dec 17, 2020

View reviewed changes

Comment thread pkg/tracing/populate_span_transformer.go Outdated

xtreme-sameer-vohra force-pushed the 4578-trace-attr branch from 26c3de1 to a7b27a4 Compare December 17, 2020 21:07

xtreme-sameer-vohra force-pushed the 4578-trace-attr branch from 20d08bb to 10bf6a0 Compare December 18, 2020 20:59

xtreme-sameer-vohra changed the title ~~WIP: Add Open Telemetry Spec attributes to IMC Dispatcher span~~ Add Open Telemetry Spec attributes to IMC Dispatcher span Dec 18, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 18, 2020

slinkydeveloper reviewed Dec 21, 2020

View reviewed changes

skonto reviewed Dec 21, 2020

View reviewed changes

zhongduo mentioned this pull request Jan 5, 2021

Add zhongduo (Jimmy Lin) as an approver #4691

Merged

slinkydeveloper reviewed Jan 7, 2021

View reviewed changes

xtreme-sameer-vohra added 4 commits January 7, 2021 09:47

[knative#4578] Update imc-dispatcher

c84b2a4

- trace should include Open Telemetry Spec attributes (messaging.*) messaging.destination messaging.message_id messaging.protocol messaging.system

Update channel tracing conformance test

9ed38da

- nit: change transformer -> mutator for consistency Signed-off-by: Sameer Vohra <vsameer@vmware.com>

xtreme-sameer-vohra force-pushed the 4578-trace-attr branch from e54a35d to 9ed38da Compare January 7, 2021 15:47

knative-prow-robot assigned slinkydeveloper Jan 12, 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. labels Jan 12, 2021

knative-prow-robot assigned vaikas Jan 14, 2021

knative-prow-robot merged commit 056bc25 into knative:master Jan 14, 2021

mattmoor mentioned this pull request Jan 14, 2021

TestChannelTracing fails #4745

Closed

		// 3. Channel Dispatcher span
		// 4. Channel sends event to Mutator pod.

Conversation

xtreme-sameer-vohra commented Dec 15, 2020

Proposed Changes

Uh oh!

xtreme-sameer-vohra commented Dec 15, 2020

Uh oh!

codecov Bot commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

matzew commented Dec 16, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhongduo commented Dec 16, 2020 via email

Uh oh!

Uh oh!

xtreme-sameer-vohra commented Dec 18, 2020

Uh oh!

xtreme-sameer-vohra commented Dec 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xtreme-sameer-vohra Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slinkydeveloper left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knative-metrics-robot commented Jan 7, 2021

Uh oh!

xtreme-sameer-vohra commented Jan 12, 2021

Uh oh!

slinkydeveloper commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Dec 15, 2020 •

edited

Loading

xtreme-sameer-vohra Dec 22, 2020 •

edited

Loading

slinkydeveloper commented Jan 12, 2021 •

edited

Loading