Add propagation to external services

[Rohith-Raju]

Fixes #385
cc @guicassolato

Verification steps courtesy @guicassolato :

Step 1: Initialize cluster

make cluster install build

Step 2: Bring up authorino

bin/authorino server --tracing-service-endpoint http://localhost:14268/api/traces

Step 3: Apply auth-config

apiVersion: authorino.kuadrant.io/v1beta1
kind: AuthConfig
metadata:
  name: talker-api-protection
spec:
  hosts:
  - talker-api-authorino.127.0.0.1.nip.io
  metadata:
  - name: echo
    http:
      endpoint: https://echo-api.3scale.net/
  response:
  - name: echo
    json:
      properties:
      - name: response
        valueFrom:
          authJSON: auth.metadata.echo

Step 4: Sending an external authorization request to a gRPC server using grpcurl

for repo in \
envoyproxy/data-plane-api \
googleapis/googleapis \
envoyproxy/protoc-gen-validate \
cncf/udpa
do
git clone git@github.com:$repo /tmp/authorino-grpc-client/$repo
done

brew install grpcurl

grpcurl -import-path /tmp/authorino-grpc-client/envoyproxy/data-plane-api  \
        -import-path /tmp/authorino-grpc-client/googleapis/googleapis \
        -import-path /tmp/authorino-grpc-client/envoyproxy/protoc-gen-validate \
        -import-path /tmp/authorino-grpc-client/cncf/udpa \
        -proto /tmp/authorino-grpc-client/envoyproxy/data-plane-api/envoy/service/auth/v3/external_auth.proto \
        -plaintext -d '{"attributes":{"request":{"http":{"host":"talker-api-authorino.127.0.0.1.nip.io"}}}}' \
        localhost:50051 \
        envoy.service.auth.v3.Authorization.Check

The response should look something like this

"status": {
    
  },
  "okResponse": {
    "headers": [
      {
        "header": {
          "key": "echo",
          "value": "{\"response\":{\"args\":\"\",\"body\":\"\",\"headers\":{\"CONTENT_TYPE\":\"text/plain\",\"HTTP_ACCEPT_ENCODING\":\"gzip\",\"HTTP_HOST\":\"echo-api.3scale.net\",\"HTTP_TRACEPARENT\":\"00-93b16549c5f367af4478a61dcdfc54e0-2a038657302c4690-01\",\"HTTP_USER_AGENT\":\"Go-http-client/1.1\",\"HTTP_VERSION\":\"HTTP/1.1\",\"HTTP_X_ENVOY_EXPECTED_RQ_TIMEOUT_MS\":\"15000\",\"HTTP_X_ENVOY_EXTERNAL_ADDRESS\":\"103.92.103.133\",\"HTTP_X_FORWARDED_FOR\":\"103.92.103.133\",\"HTTP_X_FORWARDED_PROTO\":\"https\",\"HTTP_X_REQUEST_ID\":\"be1eac9d-d925-4159-97c2-5aa235ee46bc\"},\"method\":\"GET\",\"path\":\"/\",\"uuid\":\"5e60b641-d7cd-496c-a594-ef30d5437467\"}}"
        }
      }
    ]
  },
  "dynamicMetadata": {
    }
} 

[Rohith-Raju]

Working:

In main.go we first set the propagator

otel.SetTextMapPropagator(otel_propagation.NewCompositeTextMapPropagator(otel_propagation.TraceContext{}, otel_propagation.Baggage{}))

Then use the getter to get the initialized traceContext and Baggage and inject them into the request headers

otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header))

We are injecting them where the request is being built

[Rohith-Raju]

@guicassolato Thoughts on testing this? We need to extract the information and add tracing to the external services, right?

[guicassolato]

@Rohith-Raju,

I tested the changes today. In general, it's all right. The PR does what it's expected to do, mostly.

There are just a couple things that I didn't have time to investigate further but might be worth tackling.

1. Traces are only propagated into external service calls when a tracing provider is initialised, i.e. when --tracing-service-endpoint is provided; otherwise no trace is injected. I'm not sure if this is what we want. Maybe yes. At the same time, I can't help thinking that Authorino pushing telemetry to the trace provider is one thing and propagating to external services is another, and those things should be independent from each other to an extent. Like, maybe Authorino is not emitting traces, but the external service is. In this case, Authorino would break the chain altogether (instead of just being a jump between two external spans).
2. Baggage is not being propagated into external service calls at all. This one is more concerning. Maybe it's an issue with the instrumentation library for GRPC? Or maybe it's Envoy. Anyway, this needs investigation.

I'm pretty sure these are not this PR's fault. Nevertheless, it feels we could sync this and the fixes together, maybe?

At least about # 2 above we need to be certain before merging the PR, because users will probably expect baggage to the propagated as well.

[Rohith-Raju]

Traces are only propagated into external service calls when a tracing provider is initialised, i.e. when --tracing-service-endpoint is provided; otherwise no trace is injected. I'm not sure if this is what we want. Maybe yes. At the same time, I can't help thinking that Authorino pushing telemetry to the trace provider is one thing and propagating to external services is another, and those things should be independent from each other to an extent. Like, maybe Authorino is not emitting traces, but the external service is. In this case, Authorino would break the chain altogether (instead of just being a jump between two external spans).

@guicassolato, We could introduce another flag that takes care of this or set tracing to be turned on by default unless disabled explicitly or some other way. But this could take a toll on user experience. WDYT

At least about # 2 above we need to be certain before merging the PR, because users will probably expect baggage to the propagated as well.

Yes, let's do it. 😄

[guicassolato]

We could introduce another flag that takes care of this or set tracing to be turned on by default unless disabled explicitly or some other way. But this could take a toll on user experience. WDYT

IDK if I follow 100%, but if introducing new flags + enabling tracing by default is our only option here, I prefer not to propagate traces into external service calls. Let's leave it for only when Authorino is reporting traces and this line goes back inside of the if.

[Rohith-Raju]

Baggage is not being propagated into external service calls at all. This one is more concerning. Maybe it's an issue with the instrumentation library for GRPC? Or maybe it's Envoy. Anyway, this needs investigation.

@guicassolato If your concern was whether we could propagate baggage data? Then worry no more 😄 . I manually added baggage by

bagMember, err := baggage.NewMember("baggageKey", "bagevalue")
	if err != nil {
		return nil, err
	}
	bag, err := baggage.New(bagMember)
	if err != nil {
		return nil, err
	}
	ctx = baggage.ContextWithBaggage(ctx, bag)
	otel.GetTextMapPropagator().Inject(ctx, otel_propagation.HeaderCarrier(req.Header))

And the Response

{
  "status": {
    
  },
  "okResponse": {
    "headers": [
      {
        "header": {
          "key": "echo",
          "value": "{\"response\":{\"args\":\"\",\"body\":\"\",\"headers\":{\"CONTENT_TYPE\":\"text/plain\",\"HTTP_ACCEPT_ENCODING\":\"gzip\",\"HTTP_BAGGAGE\":\"baggageKey=bagevalue\",\"HTTP_HOST\":\"echo-api.3scale.net\",\"HTTP_TRACEPARENT\":\"00-739d0ae53ea0feac8c84ce0908e02f21-24d6a23e876f319a-01\",\"HTTP_USER_AGENT\":\"Go-http-client/1.1\",\"HTTP_VERSION\":\"HTTP/1.1\",\"HTTP_X_ENVOY_EXPECTED_RQ_TIMEOUT_MS\":\"15000\",\"HTTP_X_ENVOY_EXTERNAL_ADDRESS\":\"103.92.103.133\",\"HTTP_X_FORWARDED_FOR\":\"103.92.103.133\",\"HTTP_X_FORWARDED_PROTO\":\"https\",\"HTTP_X_REQUEST_ID\":\"b193b8df-133d-4dbf-88ff-314581fbebb9\"},\"method\":\"GET\",\"path\":\"/\",\"uuid\":\"6240a839-a9a1-4108-af4a-02847480b230\"}}"
        }
      }
    ]
  },
  "dynamicMetadata": {
    }
}

Baggage is being propagated 🎉

"HTTP_BAGGAGE\":\"baggageKey=bagevalue\"

Maybe the baggage is not populated in the previous scenario? WDYT

[guicassolato]

@guicassolato If your concern was whether we could propagate baggage data? Then worry no more 😄 . I manually added baggage by

@Rohith-Raju, we need to make sure the instrumentation^1,2,3 is taking care of extracting the Baggage data automatically and injecting it into the request to the external service. If it's not, then likely the problem is in the extraction part.

¹

authorino/main.go

Line 193 in a6cc906

otel.SetTextMapPropagator(otel_propagation.NewCompositeTextMapPropagator(otel_propagation.TraceContext{}, otel_propagation.Baggage{}))

²

authorino/main.go

Lines 311 to 312 in a6cc906

	grpc.ChainStreamInterceptor(grpc_prometheus.StreamServerInterceptor, otel_grpc.StreamServerInterceptor()),
	grpc.ChainUnaryInterceptor(grpc_prometheus.UnaryServerInterceptor, otel_grpc.UnaryServerInterceptor()),

³

authorino/main.go

Line 369 in a6cc906

http.Handle(basePath, otel_http.NewHandler(handler, name))

[Rohith-Raju]

Sure, that makes sense.

[guicassolato]

@Rohith-Raju ,

I did some debugging now focusing on what's propagated by Envoy versus how instrumentation in Authorino will both extract the tracing data from the incoming requests and inject them into the outgoing ones.

TL;DR – Envoy does not inject baggage data into the requests to the external authorization service. It does inject the traceparent, but not the baggage. Both are propagated to a routed upstream service, but unfortunately only the traceparent to the external authorization.

There's a section specifically about baggage data in the Envoy docs where it says it works only for selected trace providers. However, I tried with Jaeger (where baggage should not be propagated) and with OpenTelemetry Collector (where I expected it to be propagated), but it wasn't propagated in either.

Another important finding was that tracing does not have to be enabled in Authorino for traceparents and baggage data detected in the incoming authorization requests to be injected into the outgoing ones.

I can share the steps to reproduce if you're interested.

Anyway, the bottom line is we can merge this PR. Thanks again for the hard work!

[Rohith-Raju]

@guicassolato, Thanks for merging the Pr

Another important finding was that tracing does not have to be enabled in Authorino for traceparents and baggage data detected in the incoming authorization requests to be injected into the outgoing ones.

Sure, I'd love to. I'm caught up this week with my college exams. will take it up next week