envoy icon indicating copy to clipboard operation
envoy copied to clipboard

tracing: dubbo proxy tracing #17601

Open tedli opened this issue 2 years ago • 7 comments

Commit Message: Add tracing support for dubbo proxy Additional Description: #17601 Risk Level: Low Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional API Considerations:]

tedli avatar Aug 07 '22 15:08 tedli

Hi @tedli, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/22594 was opened by tedli.

see: more, trace.

~~Up to now, my code confirmed works. It has to add code in router, because there could access decode invocation.~~ ~~I wrote a memo, describing the problem I'm facing.~~ ~~I will correct span tags and refactor code the next step.~~

This is my first PR for envoy, If I missed something, please point me out, I will correct it.

tedli avatar Aug 09 '22 09:08 tedli

Demo:

node:
  id: dev
  cluster: dev
admin:
  address:
    socket_address:
      protocol: TCP
      address: 127.0.0.1
      port_value: 15000
static_resources:
  listeners:
  - name: dubbo
    address:
      socket_address:
        address: 0.0.0.0
        port_value: 30880
    filter_chains:
    - filters:
      - name: envoy.filters.network.dubbo_proxy
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.dubbo_proxy.v3.DubboProxy
          stat_prefix: dubbo
          protocol_type: Dubbo
          serialization_type: Hessian2
          route_config:
          - name: dubbo
            interface: "*"
            routes:
            - match:
                method:
                  name:
                    safe_regex:
                      google_re2:
                        max_program_size: 100
                      regex: ".+"
              route:
                cluster: dubbo
          dubbo_filters:
          - name: envoy.filters.dubbo.router
          - name: envoy.filters.dubbo.tracer
            config:
              "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing
              provider:
                name: envoy.tracers.skywalking
                typed_config:
                  "@type": type.googleapis.com/envoy.config.trace.v3.SkyWalkingConfig
                  grpc_service:
                    envoy_grpc:
                      cluster_name: oap
                    timeout: 0.250s
                  client_config:
                    service_name: sidecar
                    instance_name: lalala
  clusters:
  - name: dubbo
    connect_timeout: 0.25s
    type: STATIC
    load_assignment:
      cluster_name: dubbo
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: 172.16.29.218
                port_value: 20880
  - name: oap
    connect_timeout: 0.25s
    type: STATIC
    typed_extension_protocol_options:
      envoy.extensions.upstreams.http.v3.HttpProtocolOptions:
        "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
        explicit_http_config:
          http2_protocol_options: {}
    load_assignment:
      cluster_name: oap
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: 172.16.29.218
                port_value: 11800

There dubbo2 sample available:

docker pull sofastack/e2e-dubbo-consumer:0.2.1
docker pull sofastack/e2e-dubbo-provider:0.2.1

I copied jars from the above images, so I can run directly on my host.

# I using powershell, if use bash or the args may need to addjust.

java -Xms1g -Xmx2g "-Ddubbo.protocol.port=20880" "-Ddubbo.provider.threadpool=fixed" "-Ddubbo.provider.threads=1000" "-Ddubbo.provider.iothreads=3" "-Dserver.port=65080" "-Dmanagement.server.port=65081" "-Dagent.service_name=provider" "-javaagent:..\..\Downloads\skywalking-agent\skywalking-agent.jar" -jar dubbo-provider.jar

java -Xms1g -Xmx2g "-Ddubbo.reference.com.alibaba.boot.dubbo.demo.consumer.DemoService.url=dubbo://172.28.216.18:30880" "-Dserver.port=65082" "-Dmanagement.server.port=65083" "-Dlogging.level.org.springframework.web=ERROR" "-Ddubbo.consumer.check=false" "-Dagent.service_name=consumer" "-javaagent:..\..\Downloads\skywalking-agent\skywalking-agent.jar" -jar dubbo-consumer.jar

And, it works.

image

tedli avatar Aug 11 '22 06:08 tedli

Thanks for you contribution. But if I miss something? Where is the new proto API for the dubbo tracing?

And there are some exist tracers which are protocol independent. We should reuse these tracers rather than to create new tracers for specific proxy.

Thanks again.

/wait

wbpcode avatar Aug 11 '22 07:08 wbpcode

Thanks for you contribution. But if I miss something? Where is the new proto API for the dubbo tracing?

And there are some exist tracers which are protocol independent. We should reuse these tracers rather than to create new tracers for specific proxy.

Thanks again.

/wait

Thanks for reply.

  1. No proto API added, this type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing proto is reused.
  2. No new tracer created. Tracing::HttpTracerSharedPtr is reused, so do the factory.

What I've done:

  1. Empty tracer filter, this filter do nothing, only holds the proto config.
  2. A tracer_util for create, manipulate span tags, and act as wrapper between header map and dubbo attachment.
  3. The skywalking driver impl is not protocol independent, the ctor of sw span hardcoded span layer to http which is the default value, so I set span layer to rpc framework.

I'm glad to finish this PR. Please point out the inappropriate code in more detail, so I can get how to correct.

Thanks.

tedli avatar Aug 11 '22 07:08 tedli

  1. IMO, it's OK to resue the HttpConnectionManager.Tracing for learning or private extension, because it just work. But if we want make this extension be part of core envoy, may be a more clear/common new proto API is necessary. Or may be we need move the HttpConnectionManager.Tracing to a common pkg.
  2. It would be better to add a new tracing filed in the DubboProxy for dubbo tracing rather than create a special L7 dubbo filter.
  3. If the skywalking driver impl is not protocol independent, we should try to fix it first.

wbpcode avatar Aug 11 '22 08:08 wbpcode

  1. IMO, it's OK to resue the HttpConnectionManager.Tracing for learning or private extension, because it just work. But if we want make this extension be part of core envoy, may be a more clear/common new proto API is necessary. Or may be we need move the HttpConnectionManager.Tracing to a common pkg.
  2. It would be better to add a new tracing filed in the DubboProxy for dubbo tracing rather than create a special L7 dubbo filter.
  3. If the skywalking driver impl is not protocol independent, we should try to fix it first.

Yes, it make sense, thanks a lot. It will be a very rewarding exercise to me.

Then hold this PR. I will firstly open up a new one to add something like setProtocol to Tracing::Span interface, so other network filters can set to corresponding protocol, rather than the default value http, to achieve protocol independent.

tedli avatar Aug 11 '22 08:08 tedli

Looks like this still needs a format fix.

/wait

jmarantz avatar Aug 15 '22 12:08 jmarantz

Looks like this still needs a format fix.

/wait

Yes. It's unfinished, I forget to convert to draft. As wbpcode said, it's better to introduce a tracing field to dubbo proxy proto, and refactor http tracing proto to common, rather than use it directly in other network filter.

I'm glad to finish this PR, I will keep on doing this.

tedli avatar Aug 15 '22 13:08 tedli

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions[bot] avatar Sep 14 '22 16:09 github-actions[bot]

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions[bot] avatar Sep 21 '22 20:09 github-actions[bot]