Added support for Micrometer Observation API #11021

marcingrzejszczak · 2022-11-24T14:20:39Z

What is the purpose of the change

Fixes #10742

Brief changelog

Updates Micrometer to 1.10.2 via a BOM
Adds Micrometer Tracing 1.0.0 via a BOM
Adds provide and consumer Observation Filters

Verifying this change

Checklist

Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
Add some description to dubbo-website project if you are requesting to add a feature.
GitHub Actions works fine on your own branch.
If this contribution is large, please follow the Software Donation Guide.

chickenlj · 2022-11-25T03:54:43Z

Could you please fix the following Checkstyle issue?

[INFO] Starting audit...
Error:  /home/runner/work/dubbo/dubbo/dubbo/dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DubboConsumerContext.java:21:8: Unused import - io.micrometer.observation.transport.Kind. [UnusedImports]
Audit done.
[INFO] There is 1 error reported by Checkstyle 8.41 with codestyle/checkstyle.xml ruleset.
Error:  src/main/java/org/apache/dubbo/metrics/filter/observation/DubboConsumerContext.java:[21,8] (imports) UnusedImports: Unused import - io.micrometer.observation.transport.Kind.
[INFO] ------------------------------------------------------------------------

chickenlj · 2022-11-25T03:55:55Z

I am still new to the Observation API, so I need to learn how it works and what advantages can it bring first.

marcingrzejszczak · 2022-11-25T07:43:22Z

Hey, sure. I first created an issue here #10742 where I described the advantages and got a green light to start working on this. Is there anything else you want me to add other than what's there in the issue?

marcingrzejszczak · 2022-11-25T11:37:41Z

Let me provide an additional explanation.

Thanks to this PR, by properly configuring Micrometer Observation (ObservationRegistry) with Micrometer Tracing (Tracer) and Micrometer Core (MeterRegistry) you can have out of the box metrics and tracing support in Apache Dubbo.

I've managed to configure that for the Boot samples in core and this is the result in Zipkin

And these are the collected metrics:

Server

==== METRICS ====
 - Metric type 	[LONG_TASK_TIMER],	name [rpc.server.duration.active],	tags [tag(net.peer.name=192.168.68.116), tag(net.peer.port=35898), tag(rpc.method=sayHello), tag(rpc.service=org.apache.dubbo.springboot.demo.DemoService), tag(rpc.system=apache_dubbo)],	measurements [Measurement{statistic='ACTIVE_TASKS', value=0.0}, Measurement{statistic='DURATION', value=0.0}]
 - Metric type 	[TIMER],	name [rpc.server.duration],	tags [tag(error=none), tag(net.peer.name=192.168.68.116), tag(net.peer.port=35898), tag(rpc.method=sayHello), tag(rpc.service=org.apache.dubbo.springboot.demo.DemoService), tag(rpc.system=apache_dubbo)],	measurements [Measurement{statistic='COUNT', value=1.0}, Measurement{statistic='TOTAL_TIME', value=0.022346498}, Measurement{statistic='MAX', value=0.022346498}]
=================

Client

==== METRICS ====
 - Metric type 	[LONG_TASK_TIMER],	name [rpc.client.duration.active],	tags [tag(rpc.method=sayHello), tag(rpc.service=org.apache.dubbo.springboot.demo.DemoService), tag(rpc.system=apache_dubbo)],	measurements [Measurement{statistic='ACTIVE_TASKS', value=0.0}, Measurement{statistic='DURATION', value=0.0}]
 - Metric type 	[TIMER],	name [rpc.client.duration],	tags [tag(error=none), tag(rpc.method=sayHello), tag(rpc.service=org.apache.dubbo.springboot.demo.DemoService), tag(rpc.system=apache_dubbo)],	measurements [Measurement{statistic='COUNT', value=1.0}, Measurement{statistic='TOTAL_TIME', value=0.188515207}, Measurement{statistic='MAX', value=0.188515207}]
=================

I did my best to follow the experimental OpenTelemetry semantic conventions for tracing and metrics

dubbo-demo/dubbo-demo-spring-boot/dubbo-demo-spring-boot-provider/pom.xml

...etrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DubboClientContext.java

...s-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java

AlbumenJ

Please merge this PR into 3.2 branch for new feature purpose.

AlbumenJ

BTW, a little question. Could be configuration in dubbo-demo be more simplify?

sonarqubecloud · 2022-11-29T10:18:09Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
12 Code Smells

43.1% Coverage
18.2% Duplication

marcingrzejszczak · 2022-11-29T12:02:05Z

I can remove the Zipkin dependency from the samples - this will simplify things a little bit. However, if you migrate dubbo to use Spring Boot 3, then most of the configuration will be gone because it's done behind the scenes by Boot.

marcingrzejszczak · 2022-11-29T14:51:47Z

Rebased my branch on top of 3.2 of dubbo

marcingrzejszczak · 2022-11-30T09:07:29Z

Ok I fixed the provider side, what else should I do to have this PR merged?

AlbumenJ · 2022-12-05T03:21:58Z

@CrazyHZM PTAL

CrazyHZM · 2022-12-05T03:39:36Z

Thanks to @marcingrzejszczak for this contribution, because the content of this PR will directly provide users with the Micrometer Observation function, we need to discuss whether it will be the default implementation of Dubbo, or temporarily used as an SPI EXTENSION(https://github.com/apache/dubbo-spi-extensions). Please give us some time to understand the Micrometer Observation function

marcingrzejszczak · 2022-12-05T11:00:48Z

Sure, if you have any questions I'll be more than happy to answer them.

conghuhu · 2023-01-08T12:54:48Z

...ava/org/apache/dubbo/metrics/filter/observation/DefaultDubboServerObservationConvention.java

+/**
+ * Default implementation of the {@link DubboConsumerObservationConvention}.
+ */
+public class DefaultDubboServerObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboConsumerObservationConvention {


Hi, I think DefaultDubboServerObservationConvention should implements DubboProviderObservationConvention, not DubboConsumerObservationConvention

To tell you the truth I'm completely confused with how dubbo understands receiver and sender (aka client and server aka producer consumer). I think that with this setup I've managed to get proper results in Zipkin (which doesn't mean that I've done this properly)

Yes, there is no problem with the function, but it may prefer client as consumer semantically

I've changed the names to Server and Client instead of Provider and Consumer

conghuhu · 2023-01-08T12:55:28Z

...ava/org/apache/dubbo/metrics/filter/observation/DefaultDubboClientObservationConvention.java

+/**
+ * Default implementation of the {@link DubboProviderObservationConvention}.
+ */
+public class DefaultDubboClientObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboProviderObservationConvention {


Hi, I think DefaultDubboClientObservationConvention should implements DubboConsumerObservationConvention, not DubboProviderObservationConvention

conghuhu · 2023-01-08T14:32:33Z

...src/main/resources/META-INF/dubbo/internal/org.apache.dubbo.rpc.cluster.filter.ClusterFilter

@@ -0,0 +1,2 @@
+observationreceiver=org.apache.dubbo.metrics.filter.observation.ObservationReceiverFilter


ObservationReceiverFilter is implements Filter, so it needs to be placed in org.apache.dubbo.rpc.Filter, or the ObservationReceiverFilter will not take effect.
When I tested locally, I found that the invoke method of the filter was not called.

See the comments below I made on Filter and ClusterFilter

This needs to be changed, because ObservationReceiverFilter is implements Filter.
As shown in the figure, put it in the configuration file of org.apache.dubbo.rpc.Filter

This needs to be changed, because ObservationReceiverFilter is implements Filter. As shown in the figure, put it in the configuration file of org.apache.dubbo.rpc.Filter

Would implementing org.apache.dubbo.rpc.ClusterFilter be better?

org.apache.dubbo.rpc.ClusterFilter should be applied to consumer, but ObservationReceiverFilter is provider.
So ObservationReceiverFilter shoule implement Filter

...s-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java

CrazyHZM · 2023-01-17T03:13:25Z

ping @marcingrzejszczak
Please check and process the above review content.

marcingrzejszczak · 2023-01-17T08:49:43Z

Yup, I will, thanks :)

conghuhu · 2023-01-17T08:57:03Z

...i/src/test/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilterTest.java

+
+            filter.invoke(invoker, invocation);
+
+            BDDAssertions.then(RpcContext.getClientAttachment().getAttachment("X-B3-TraceId")).isNotEmpty();


Unit test is error in here.
I think DubboClientContext and DubboServerContext should be changed as follows:

...src/main/resources/META-INF/dubbo/internal/org.apache.dubbo.rpc.cluster.filter.ClusterFilter

CrazyHZM · 2023-01-18T09:36:19Z

@marcingrzejszczak
Hello, we are actively promoting Dubbo's observability capabilities, including tracing. We have also reviewed the PR and found that there is no problem with the basic content, but there are some problems with the details. These problems may be related to Dubbo's principle.
In addition, we have other new features that need to be iterated, and they are based on this PR. Therefore, interested partners in the community have already solved the problem of this PR for you, and we also plan to merge this PR, then @conghuhu will provide a PR to fix the problem on this PR. Welcome to review that PR at that time, thank you.

marcingrzejszczak · 2023-01-18T10:58:09Z

Great to hear that! I'll try to review this today and then feel free to merge it :)

marcingrzejszczak · 2023-01-18T12:23:13Z

So should I update the PR with your suggestions or I shouldn't touch it at all and just do a review?

CrazyHZM · 2023-01-18T12:31:28Z

So should I update the PR with your suggestions or I shouldn't touch it at all and just do a review?

It would of course be best if the issue could be fixed on this PR in time.
@marcingrzejszczak

codecov-commenter · 2023-01-23T13:21:10Z

Codecov Report

Merging #11021 (fca5473) into 3.2 (d28b224) will decrease coverage by 4.83%.
The diff coverage is 72.65%.

@@             Coverage Diff              @@
##                3.2   #11021      +/-   ##
============================================
- Coverage     69.62%   64.80%   -4.83%     
+ Complexity      131       14     -117     
============================================
  Files          1515     1497      -18     
  Lines         82452    62498   -19954     
  Branches      14757     9146    -5611     
============================================
- Hits          57408    40499   -16909     
+ Misses        20201    17763    -2438     
+ Partials       4843     4236     -607

Impacted Files	Coverage Δ
...ion/AbstractDefaultDubboObservationConvention.java	`38.88% <38.88%> (ø)`
...ation/DefaultDubboClientObservationConvention.java	`60.00% <60.00%> (ø)`
.../filter/observation/ObservationReceiverFilter.java	`73.91% <73.91%> (ø)`
...cs/filter/observation/ObservationSenderFilter.java	`82.60% <82.60%> (ø)`
...metrics/filter/observation/DubboClientContext.java	`85.71% <85.71%> (ø)`
...metrics/filter/observation/DubboServerContext.java	`85.71% <85.71%> (ø)`
...o/metrics/filter/observation/DubboObservation.java	`88.88% <88.88%> (ø)`
...ation/DefaultDubboServerObservationConvention.java	`100.00% <100.00%> (ø)`
.../observation/DubboClientObservationConvention.java	`100.00% <100.00%> (ø)`
.../observation/DubboServerObservationConvention.java	`100.00% <100.00%> (ø)`
... and 488 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

AlbumenJ

LGTM

AlbumenJ · 2023-01-24T13:08:46Z

@marcingrzejszczak Thanks for your kindly contribution. I will merge it later until CI works fine.
After this PR, we still have some things to improve on the metrics part.

For the ObservationConfiguration class provided in the demo project in this PR, we need a detailed document to explain its details and guide users how to configure it in their own Spring or SpringBoot projects. In addition, can we provide a SpringBoot Starter to simplify the configuration difficulty for users.
The current docking is based on the micrometer. For user production deployment, we also need to provide a feasible and sufficiently detailed solution for reporting to zipkin or wavefront through OTEL.

marcingrzejszczak · 2023-01-24T13:59:14Z

For the ObservationConfiguration class provided in the demo project in this PR, we need a detailed document to explain its details and guide users how to configure it in their own Spring or SpringBoot projects. In addition, can we provide a SpringBoot Starter to simplify the configuration difficulty for users.

Actually the whole setup is done manually only because you're not using Boot 3. In Boot 3 all of that is auto-configured.

The current docking is based on the micrometer. For user production deployment, we also need to provide a feasible and sufficiently detailed solution for reporting to zipkin or wavefront through OTEL.

Micrometer Observation can work with Micrometer Tracing. Micrometer Tracing is an abstraction over tracing that can use OTEL as a bridge. There's nothing you need to change really to make this work with OTel 🤷 BTW you can find this here https://github.com/apache/dubbo/pull/11021/files#diff-dfd2e12c668d7d1cb35baf3117d32272b38a03e0bec26dbd21a324098d90783dR138-R145

AlbumenJ · 2023-01-24T14:11:53Z

Some test cases need to be fixed.

AlbumenJ · 2023-01-24T14:23:40Z

Actually the whole setup is done manually only because you're not using Boot 3. In Boot 3 all of that is auto-configured.

I have created some spring boot 3 related samples[1] for Dubbo last month. Do you mean that in these samples everything will work fine without any configuration?

[1] https://github.com/apache/dubbo-samples/tree/master/10-task

Micrometer Observation can work with Micrometer Tracing. Micrometer Tracing is an abstraction over tracing that can use OTEL as a bridge. There's nothing you need to change really to make this work with OTel 🤷 BTW you can find this here https://github.com/apache/dubbo/pull/11021/files#diff-dfd2e12c668d7d1cb35baf3117d32272b38a03e0bec26dbd21a324098d90783dR138-R145

Are you willing to submit relevant demos and documents about this function? It is still very difficult for users who are not familiar with us to use it. 🤯

fixes apachegh-10742

…/metrics/filter/observation/ObservationSenderFilter.java Co-authored-by: Albumen Kevin <[email protected]>

…/metrics/filter/observation/DubboClientContext.java Co-authored-by: Albumen Kevin <[email protected]>

…to context attachment

marcingrzejszczak · 2023-01-24T15:36:14Z

Fixed the broken test, sorry about that

marcingrzejszczak · 2023-01-24T15:38:18Z

I have created some spring boot 3 related samples[1] for Dubbo last month. Do you mean that in these samples everything will work fine without any configuration?

If you add the dependencies as presented here then yes, you don't need to pass any configuration. Check this doc https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#actuator.micrometer-tracing and https://micrometer.io/docs/tracing

Are you willing to submit relevant demos and documents about this function? It is still very difficult for users who are not familiar with us to use it. exploding_head

Sure, how can I help?

sonarqubecloud · 2023-01-25T01:40:49Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
14 Code Smells

42.8% Coverage
23.2% Duplication

AlbumenJ · 2023-01-25T13:56:20Z

Sure, how can I help?

Can you help fill in the documentation for this article?
https://dubbo.apache.org/en/docs3-v2/java-sdk/advanced-features-and-usage/observability/tracing/
https://github.com/apache/dubbo-website/edit/master/content/en/docs3-v2/java-sdk/advanced-features-and-usage/observability/tracing.md

marcingrzejszczak · 2023-01-26T12:59:35Z

there you go apache/dubbo-website#1954

marcingrzejszczak mentioned this pull request Nov 24, 2022

Micrometer Observation Support #10742

Closed

2 tasks

marcingrzejszczak force-pushed the micrometerObservation branch from d57460e to 7460342 Compare November 25, 2022 09:18

marcingrzejszczak force-pushed the micrometerObservation branch from 7460342 to 9b671da Compare November 25, 2022 11:59

AlbumenJ requested changes Nov 29, 2022

View reviewed changes

AlbumenJ reviewed Nov 29, 2022

View reviewed changes

marcingrzejszczak changed the base branch from 3.1 to 3.2 November 29, 2022 14:50

marcingrzejszczak force-pushed the micrometerObservation branch from b549a3f to 332eb19 Compare November 29, 2022 14:51

AlbumenJ requested a review from CrazyHZM December 5, 2022 03:22

conghuhu reviewed Jan 11, 2023

View reviewed changes

conghuhu reviewed Jan 17, 2023

View reviewed changes

...src/main/resources/META-INF/dubbo/internal/org.apache.dubbo.rpc.cluster.filter.ClusterFilter Show resolved Hide resolved

marcingrzejszczak force-pushed the micrometerObservation branch from e47c1d3 to 60cb6fe Compare January 18, 2023 14:07

AlbumenJ approved these changes Jan 24, 2023

View reviewed changes

marcingrzejszczak and others added 15 commits January 24, 2023 15:50

Added support for Micrometer Observation API

25fb177

fixes apachegh-10742

Applied changes following ther eview

8f0d3e2

Fixed the filter on the consumer side

350725b

Applied changes following the review

89c6e04

Upgraded micrometer dependencies

9adc502

Fixed checkstyle issues

dc9a287

Update dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo…

64f9e5c

…/metrics/filter/observation/ObservationSenderFilter.java Co-authored-by: Albumen Kevin <[email protected]>

Update dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo…

ed500b3

…/metrics/filter/observation/DubboClientContext.java Co-authored-by: Albumen Kevin <[email protected]>

Changes following review

bc1de0a

Uses RpcUtils for method resolution

3d18c23

Fixed checksyle

fd54bc6

Fixed checksyle

bc7c214

Trying to retrieve the port and host in a different way and fallback …

26c2feb

…to context attachment

Iterating over invoked invokers

0550c08

Fixed broken test

fca5473

marcingrzejszczak force-pushed the micrometerObservation branch from 20ee4a1 to fca5473 Compare January 24, 2023 15:36

AlbumenJ merged commit b456d23 into apache:3.2 Jan 25, 2023

seriouszyx mentioned this pull request Jan 28, 2023

Observability tasks: provider and consumer tracing data buried points(可观测性任务：提供者与消费者tracing 数据埋点) #11171

Closed

2 tasks

ShenFeng312 mentioned this pull request Feb 16, 2023

add dubbo-spring-boot-observability-starter #11579

Merged

8 tasks

		@@ -0,0 +1,2 @@
		observationreceiver=org.apache.dubbo.metrics.filter.observation.ObservationReceiverFilter


		filter.invoke(invoker, invocation);

		BDDAssertions.then(RpcContext.getClientAttachment().getAttachment("X-B3-TraceId")).isNotEmpty();

Added support for Micrometer Observation API #11021

Added support for Micrometer Observation API #11021

Conversation

marcingrzejszczak commented Nov 24, 2022

What is the purpose of the change

Brief changelog

Verifying this change

Checklist

chickenlj commented Nov 25, 2022

chickenlj commented Nov 25, 2022

marcingrzejszczak commented Nov 25, 2022

marcingrzejszczak commented Nov 25, 2022 • edited Loading

AlbumenJ left a comment

Choose a reason for hiding this comment

AlbumenJ left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 29, 2022

marcingrzejszczak commented Nov 29, 2022

marcingrzejszczak commented Nov 29, 2022

marcingrzejszczak commented Nov 30, 2022

AlbumenJ commented Dec 5, 2022

CrazyHZM commented Dec 5, 2022

marcingrzejszczak commented Dec 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CrazyHZM commented Jan 17, 2023 • edited Loading

marcingrzejszczak commented Jan 17, 2023

Choose a reason for hiding this comment

CrazyHZM commented Jan 18, 2023

marcingrzejszczak commented Jan 18, 2023

marcingrzejszczak commented Jan 18, 2023

CrazyHZM commented Jan 18, 2023

codecov-commenter commented Jan 23, 2023 • edited Loading

Codecov Report

AlbumenJ left a comment

Choose a reason for hiding this comment

AlbumenJ commented Jan 24, 2023

marcingrzejszczak commented Jan 24, 2023

AlbumenJ commented Jan 24, 2023

AlbumenJ commented Jan 24, 2023

marcingrzejszczak commented Jan 24, 2023

marcingrzejszczak commented Jan 24, 2023

sonarqubecloud bot commented Jan 25, 2023

AlbumenJ commented Jan 25, 2023

marcingrzejszczak commented Jan 26, 2023 • edited Loading

marcingrzejszczak commented Nov 25, 2022 •

edited

Loading

CrazyHZM commented Jan 17, 2023 •

edited

Loading

codecov-commenter commented Jan 23, 2023 •

edited

Loading

marcingrzejszczak commented Jan 26, 2023 •

edited

Loading