You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2022/11/24 14:20:41 UTC
[GitHub] [dubbo] marcingrzejszczak opened a new pull request, #11021: Added support for Micrometer Observation API
marcingrzejszczak opened a new pull request, #11021:
URL: https://github.com/apache/dubbo/pull/11021
## What is the purpose of the change
Fixes https://github.com/apache/dubbo/issues/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
<!-- Follow this checklist to help us incorporate your contribution quickly and easily: -->
## Checklist
- [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) 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.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
- [x] 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](https://github.com/apache/dubbo-samples) project.
- [ ] Add some description to [dubbo-website](https://github.com/apache/dubbo-website) project if you are requesting to add a feature.
- [ ] GitHub Actions works fine on your own branch.
- [x] If this contribution is large, please follow the [Software Donation Guide](https://github.com/apache/dubbo/wiki/Software-donation-guide).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by "marcingrzejszczak (via GitHub)" <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1084090540
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboClientObservationConvention.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+
+/**
+ * Default implementation of the {@link DubboClientObservationConvention}.
+ */
+public class DefaultDubboClientObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboClientObservationConvention {
+ /**
+ * Singleton instance of {@link DefaultDubboClientObservationConvention}.
+ */
+ public static final DubboClientObservationConvention INSTANCE = new DefaultDubboClientObservationConvention();
+
+ @Override
+ public String getName() {
+ return "rpc.client.duration";
+ }
+
+ @Override
+ public KeyValues getLowCardinalityKeyValues(DubboClientContext context) {
+ KeyValues keyValues = super.getLowCardinalityKeyValues(context.getInvocation());
+ RpcContextAttachment rpcContextAttachment = context.getRpcContextAttachment();
+ URL url = context.getInvoker().getUrl();
Review Comment:
I see. So what I did for now is to attach tags only from the first invoker. We could think of adding more tags for additional ones later once there's demand for this, WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1401922420
@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.
1. 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.
2. 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1327116896
Hey, sure. I first created an issue here https://github.com/apache/dubbo/issues/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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1336689722
@CrazyHZM PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1064138275
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboServerObservationConvention.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+
+/**
+ * Default implementation of the {@link DubboConsumerObservationConvention}.
+ */
+public class DefaultDubboServerObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboConsumerObservationConvention {
Review Comment:
Hi, I think `DefaultDubboServerObservationConvention` should implements `DubboProviderObservationConvention`, not `DubboConsumerObservationConvention`
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboClientObservationConvention.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+
+/**
+ * Default implementation of the {@link DubboProviderObservationConvention}.
+ */
+public class DefaultDubboClientObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboProviderObservationConvention {
Review Comment:
Hi, I think `DefaultDubboClientObservationConvention` should implements `DubboConsumerObservationConvention`, not `DubboProviderObservationConvention`
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
Review Comment:
I have a question, why does `ObservationSenderFilter` implements `ClusterFilter` not `Filter`.
Are there any special considerations？
##########
dubbo-metrics/dubbo-metrics-api/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
Review Comment:
`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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] codecov-commenter commented on pull request #11021: Added support for Micrometer Observation API
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1400330030
# [Codecov](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#11021](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1043894) into [3.2](https://codecov.io/gh/apache/dubbo/commit/54bb2765a72d3faa76bb971b445c8e496c69decb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (54bb276) will **decrease** coverage by `4.80%`.
> The diff coverage is `74.60%`.
```diff
@@ Coverage Diff @@
## 3.2 #11021 +/- ##
============================================
- Coverage 69.61% 64.82% -4.80%
+ Complexity 133 14 -119
============================================
Files 1515 1497 -18
Lines 82452 62496 -19956
Branches 14757 9145 -5612
============================================
- Hits 57402 40513 -16889
+ Misses 20210 17744 -2466
+ Partials 4840 4239 -601
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ion/AbstractDefaultDubboObservationConvention.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vQWJzdHJhY3REZWZhdWx0RHViYm9PYnNlcnZhdGlvbkNvbnZlbnRpb24uamF2YQ==) | `38.88% <38.88%> (ø)` | |
| [...ation/DefaultDubboClientObservationConvention.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vRGVmYXVsdER1YmJvQ2xpZW50T2JzZXJ2YXRpb25Db252ZW50aW9uLmphdmE=) | `60.00% <60.00%> (ø)` | |
| [.../filter/observation/ObservationReceiverFilter.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vT2JzZXJ2YXRpb25SZWNlaXZlckZpbHRlci5qYXZh) | `73.91% <73.91%> (ø)` | |
| [...cs/filter/observation/ObservationSenderFilter.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vT2JzZXJ2YXRpb25TZW5kZXJGaWx0ZXIuamF2YQ==) | `83.33% <83.33%> (ø)` | |
| [...metrics/filter/observation/DubboServerContext.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vRHViYm9TZXJ2ZXJDb250ZXh0LmphdmE=) | `85.71% <85.71%> (ø)` | |
| [...o/metrics/filter/observation/DubboObservation.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vRHViYm9PYnNlcnZhdGlvbi5qYXZh) | `88.88% <88.88%> (ø)` | |
| [...ation/DefaultDubboServerObservationConvention.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vRGVmYXVsdER1YmJvU2VydmVyT2JzZXJ2YXRpb25Db252ZW50aW9uLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...metrics/filter/observation/DubboClientContext.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vRHViYm9DbGllbnRDb250ZXh0LmphdmE=) | `100.00% <100.00%> (ø)` | |
| [.../observation/DubboClientObservationConvention.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vRHViYm9DbGllbnRPYnNlcnZhdGlvbkNvbnZlbnRpb24uamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [.../observation/DubboServerObservationConvention.java](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0cmljcy9kdWJiby1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vbWV0cmljcy9maWx0ZXIvb2JzZXJ2YXRpb24vRHViYm9TZXJ2ZXJPYnNlcnZhdGlvbkNvbnZlbnRpb24uamF2YQ==) | `100.00% <100.00%> (ø)` | |
| ... and [490 more](https://codecov.io/gh/apache/dubbo/pull/11021?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ merged pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ merged PR #11021:
URL: https://github.com/apache/dubbo/pull/11021
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1084081951
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboClientObservationConvention.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+
+/**
+ * Default implementation of the {@link DubboClientObservationConvention}.
+ */
+public class DefaultDubboClientObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboClientObservationConvention {
+ /**
+ * Singleton instance of {@link DefaultDubboClientObservationConvention}.
+ */
+ public static final DubboClientObservationConvention INSTANCE = new DefaultDubboClientObservationConvention();
+
+ @Override
+ public String getName() {
+ return "rpc.client.duration";
+ }
+
+ @Override
+ public KeyValues getLowCardinalityKeyValues(DubboClientContext context) {
+ KeyValues keyValues = super.getLowCardinalityKeyValues(context.getInvocation());
+ RpcContextAttachment rpcContextAttachment = context.getRpcContextAttachment();
+ URL url = context.getInvoker().getUrl();
Review Comment:
The behavior of retrying is a framework behavior. For our users, it's actually a request, just replayed.
For example, Dubbo supports automatically requesting another provider when it finds a network exception of one provider. The addresses of the two providers may be completely different.
Or Dubbo also supports broadcast mode invoke, that is, an actual request is sent to all providers in a manner similar to satellite broadcasting.
These may cause more than one invoked invoker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1331841989
Ok I fixed the provider side, what else should I do to have this PR merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] CrazyHZM commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
CrazyHZM commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1384781003
ping @marcingrzejszczak
Pleas check and process the above review content.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1397143566
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=11021)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list) [40.7% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list) [23.6% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1081448498
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
I would like to retrieve the remote port and the remote host in the consumer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1402033027
> 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. 🤯
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by "marcingrzejszczak (via GitHub)" <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1402155033
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1386969270
So should I update the PR with your suggestions or I shouldn't touch it at all and just do a review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1073580976
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboServerObservationConvention.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+
+/**
+ * Default implementation of the {@link DubboConsumerObservationConvention}.
+ */
+public class DefaultDubboServerObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboConsumerObservationConvention {
Review Comment:
I've changed the names to `Server` and `Client` instead of `Provider` and `Consumer`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1386863695
Great to hear that! I'll try to review this today and then feel free to merge it :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1387132015
I've applied the changes following the review and rebased against 3.2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1330520414
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #11021: Added support for Micrometer Observation API
Posted by sonarcloud.
sonarcloud[bot] commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1402929693
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=11021)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list) [42.8% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list) [23.2% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] chickenlj commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
chickenlj commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1326997223
I am still new to the Observation API, so I need to learn how it works and what advantages can it bring first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1073443101
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboServerObservationConvention.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+
+/**
+ * Default implementation of the {@link DubboConsumerObservationConvention}.
+ */
+public class DefaultDubboServerObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboConsumerObservationConvention {
Review Comment:
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)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1082079313
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
I have been submit a [PR](https://github.com/apache/dubbo/pull/11359) related this feature, which may helps you to retrieve the invoked remote invoker in `ClusterFilter#onResponse` and `ClusterFilter#onError`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1084013190
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboClientObservationConvention.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+
+/**
+ * Default implementation of the {@link DubboClientObservationConvention}.
+ */
+public class DefaultDubboClientObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboClientObservationConvention {
+ /**
+ * Singleton instance of {@link DefaultDubboClientObservationConvention}.
+ */
+ public static final DubboClientObservationConvention INSTANCE = new DefaultDubboClientObservationConvention();
+
+ @Override
+ public String getName() {
+ return "rpc.client.duration";
+ }
+
+ @Override
+ public KeyValues getLowCardinalityKeyValues(DubboClientContext context) {
+ KeyValues keyValues = super.getLowCardinalityKeyValues(context.getInvocation());
+ RpcContextAttachment rpcContextAttachment = context.getRpcContextAttachment();
+ URL url = context.getInvoker().getUrl();
Review Comment:
```suggestion
List<Invoker<?>> invokedInvokers = context.getInvocation().getInvokedInvokers();
for (Invoker<?> invokedInvoker : invokedInvokers) {
URL url = invokedInvoker.getUrl();
// do something you want to
}
```
Be notice that there may more than one invoker being invoked in one time. E.g. Dubbo support retry for several times when invoke failed.
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DubboClientContext.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import java.util.Objects;
+
+import io.micrometer.observation.transport.SenderContext;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+/**
+ * Provider context for RPC.
+ */
+public class DubboClientContext extends SenderContext<Invocation> {
+
+ private final Invoker<?> invoker;
+
+ private final Invocation invocation;
+
+ private final RpcContextAttachment rpcContextAttachment;
+
+ public DubboClientContext(RpcContextAttachment rpcContextAttachment, Invoker<?> invoker, Invocation invocation) {
+ super((map, key, value) -> Objects.requireNonNull(map).setAttachment(key, value));
+ this.invoker = invoker;
+ this.invocation = invocation;
+ this.rpcContextAttachment = rpcContextAttachment;
+ setCarrier(invocation);
+ }
+
+ public RpcContextAttachment getRpcContextAttachment() {
+ return rpcContextAttachment;
+ }
Review Comment:
Remove this field. You can get RpcContext anywhere you need by TL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by "marcingrzejszczak (via GitHub)" <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1404970637
there you go https://github.com/apache/dubbo-website/compare/master...marcingrzejszczak:dubbo-website:patch-1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1402014397
Some test cases need to be fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1035000839
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ *
+ * @author Marcin Grzejszczak
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements Filter, BaseFilter.Listener, ScopeModelAware {
Review Comment:
Oops i think i did that for the other filter too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1330399706
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=11021)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [12 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list) [43.1% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list) [18.2% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1330769570
Rebased my branch on top of 3.2 of dubbo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1066569087
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
Review Comment:
ok, that's my mistake
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1084016634
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
I have submitted the suggest changes below.
In addition, I haven't figured out how to record this record if Dubbo's request occurs many times.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1073552298
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboServerObservationConvention.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+
+/**
+ * Default implementation of the {@link DubboConsumerObservationConvention}.
+ */
+public class DefaultDubboServerObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboConsumerObservationConvention {
Review Comment:
Yes, there is no problem with the function, but it may prefer client as consumer semantically
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1327368184
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](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md) and [metrics](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc-metrics.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1034515995
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DubboClientContext.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import java.util.Map;
+import java.util.Objects;
+
+import io.micrometer.observation.transport.SenderContext;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+/**
+ * Provider context for RPC.
+ *
+ * @author Marcin Grzejszczak
Review Comment:
Please remove this tag due to the apache way
##########
dubbo-demo/dubbo-demo-spring-boot/dubbo-demo-spring-boot-provider/pom.xml:
##########
@@ -133,6 +132,26 @@
<artifactId>log4j</artifactId>
</dependency>
+ <!-- Observability -->
+ <dependency>
+ <groupId>org.apache.dubbo</groupId>
+ <artifactId>dubbo-metrics-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.micrometer</groupId>
+ <artifactId>micrometer-tracing-bridge-otel</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-zipkin</artifactId>
+ <version>1.20.0</version>
Review Comment:
Version should be managed in `dubbo-dependencies-bom`
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ *
+ * @author Marcin Grzejszczak
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements Filter, BaseFilter.Listener, ScopeModelAware {
Review Comment:
```suggestion
public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
```
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ *
+ * @author Marcin Grzejszczak
+ */
+@Activate(group = CONSUMER, order = -1)
Review Comment:
Should we split this filter into two filter? One for time span collection.(The order could be the lowest, and will be executed more early.) Another for attachment collection.(The order could be the highest, and to get all attachment set by other filters.)
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ *
+ * @author Marcin Grzejszczak
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements Filter, BaseFilter.Listener, ScopeModelAware {
+
+ private ObservationRegistry observationRegistry = ObservationRegistry.NOOP;
+
+ private DubboProviderObservationConvention providerObservationConvention = null;
+
+ @Override
+ public void setApplicationModel(ApplicationModel applicationModel) {
+ observationRegistry = applicationModel.getBeanFactory().getBean(ObservationRegistry.class);
+ providerObservationConvention = applicationModel.getBeanFactory().getBean(DubboProviderObservationConvention.class);
+ }
+
+ @Override
+ public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
+ if (observationRegistry == null) {
+ return invoker.invoke(invocation);
+ }
+ RpcContextAttachment context = RpcContext.getClientAttachment();
+ DubboClientContext senderContext = new DubboClientContext(context, invoker, invocation);
Review Comment:
A better to fetch attachment is from `invocation.getAttachments()`
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ *
+ * @author Marcin Grzejszczak
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements Filter, BaseFilter.Listener, ScopeModelAware {
Review Comment:
BTW, change the spi definition to META-INF/dubbo/internal/org.apache.dubbo.rpc.cluster.filter.ClusterFilter
Note: only for consumer filter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] CrazyHZM commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
CrazyHZM commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1386979430
> 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1073545888
##########
dubbo-metrics/dubbo-metrics-api/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
+observationsender=org.apache.dubbo.metrics.filter.observation.ObservationSenderFilter
Review Comment:
```
➜ dubbo-metrics git:(micrometerObservation) tree
.
├── dubbo-metrics-api
│ ├── pom.xml
│ └── src
│ ├── main
│ │ ├── java
│ │ │ └── org
│ │ │ └── apache
│ │ │ └── dubbo
│ │ │ └── metrics
│ │ │ ├── AbstractMetricsReporterFactory.java
│ │ │ ├── AbstractMetricsReporter.java
│ │ │ ├── aggregate
│ │ │ │ ├── TimeWindowCounter.java
│ │ │ │ └── TimeWindowQuantile.java
│ │ │ ├── collector
│ │ │ │ └── AggregateMetricsCollector.java
│ │ │ ├── DubboMetrics.java
│ │ │ ├── filter
│ │ │ │ ├── MetricsCollectExecutor.java
│ │ │ │ ├── MetricsFilter.java
│ │ │ │ └── observation
│ │ │ │ ├── AbstractDefaultDubboObservationConvention.java
│ │ │ │ ├── DefaultDubboClientObservationConvention.java
│ │ │ │ ├── DefaultDubboServerObservationConvention.java
│ │ │ │ ├── DubboClientContext.java
│ │ │ │ ├── DubboConsumerObservationConvention.java
│ │ │ │ ├── DubboObservation.java
│ │ │ │ ├── DubboProviderObservationConvention.java
│ │ │ │ ├── DubboServerContext.java
│ │ │ │ ├── ObservationReceiverFilter.java
│ │ │ │ └── ObservationSenderFilter.java
│ │ │ └── service
│ │ │ └── DefaultMetricsService.java
│ │ └── resources
│ │ └── META-INF
│ │ └── dubbo
│ │ └── internal
│ │ ├── org.apache.dubbo.common.metrics.service.MetricsService
│ │ └── org.apache.dubbo.rpc.cluster.filter.ClusterFilter
```
This looks good to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] CrazyHZM commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
CrazyHZM commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1080846615
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
Review Comment:
There is a `Generic` scenario here, you need to use the `RpcUtils#getMethodName() `method to get
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] CrazyHZM commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
CrazyHZM commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1386753347
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #11021: Added support for Micrometer Observation API
Posted by sonarcloud.
sonarcloud[bot] commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1400323990
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=11021)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list) [43.1% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list) [23.3% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] chickenlj commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
chickenlj commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1066539834
##########
dubbo-metrics/dubbo-metrics-api/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
Review Comment:
See the comments below I made on Filter and ClusterFilter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] chickenlj commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
chickenlj commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1069588011
##########
dubbo-metrics/dubbo-metrics-api/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
Review Comment:
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1081084317
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
Good catch, that's for the client https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#http-client
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1081346258
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
> I've added a commit that addresses most of the suggestions. The only problem is with this part here and I left it as it was cause I don't know how to retrieve this information in any other way
Do you mean that you want to retrieve the remote port in consumer?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1085280644
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboClientObservationConvention.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+
+/**
+ * Default implementation of the {@link DubboClientObservationConvention}.
+ */
+public class DefaultDubboClientObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboClientObservationConvention {
+ /**
+ * Singleton instance of {@link DefaultDubboClientObservationConvention}.
+ */
+ public static final DubboClientObservationConvention INSTANCE = new DefaultDubboClientObservationConvention();
+
+ @Override
+ public String getName() {
+ return "rpc.client.duration";
+ }
+
+ @Override
+ public KeyValues getLowCardinalityKeyValues(DubboClientContext context) {
+ KeyValues keyValues = super.getLowCardinalityKeyValues(context.getInvocation());
+ RpcContextAttachment rpcContextAttachment = context.getRpcContextAttachment();
+ URL url = context.getInvoker().getUrl();
Review Comment:
Agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1071927912
##########
dubbo-metrics/dubbo-metrics-api/src/test/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilterTest.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.core.tck.MeterRegistryAssert;
+import io.micrometer.tracing.test.simple.SpansAssert;
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.assertj.core.api.BDDAssertions;
+
+class ObservationSenderFilterTest extends AbstractObservationFilterTest {
+
+ @Override
+ public SampleTestRunnerConsumer yourCode() throws Exception {
+ return (buildingBlocks, meterRegistry) -> {
+ setupConfig();
+ setupAttachments();
+
+ filter.invoke(invoker, invocation);
+
+ BDDAssertions.then(RpcContext.getClientAttachment().getAttachment("X-B3-TraceId")).isNotEmpty();
Review Comment:
Unit test is error in here.
I think DubboClientContext and DubboServerContext should be changed as follows:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on pull request #11021: Added support for Micrometer Observation API
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1403663357
> 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] CrazyHZM commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
CrazyHZM commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1336702221
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] chickenlj commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
chickenlj commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1326996816
Could you please fix the following Checkstyle issue?
```java
[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] ------------------------------------------------------------------------
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1034856117
##########
dubbo-demo/dubbo-demo-spring-boot/dubbo-demo-spring-boot-provider/pom.xml:
##########
@@ -133,6 +132,26 @@
<artifactId>log4j</artifactId>
</dependency>
+ <!-- Observability -->
+ <dependency>
+ <groupId>org.apache.dubbo</groupId>
+ <artifactId>dubbo-metrics-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.micrometer</groupId>
+ <artifactId>micrometer-tracing-bridge-otel</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-zipkin</artifactId>
+ <version>1.20.0</version>
Review Comment:
I removed that to simplify the example
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1070206904
##########
dubbo-metrics/dubbo-metrics-api/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
Review Comment:
`org.apache.dubbo.rpc.ClusterFilter` should be applied to consumer, but `ObservationReceiverFilter` is provider.
So `ObservationReceiverFilter` shoule implement `Filter`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1385038819
Yup, I will, thanks :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1071941307
##########
dubbo-metrics/dubbo-metrics-api/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
+observationsender=org.apache.dubbo.metrics.filter.observation.ObservationSenderFilter
Review Comment:
I found some problems with the location of the resources folder:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1081221987
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
I've added a commit that addresses most of the suggestions. The only problem is with this part here and I left it as it was cause I don't know how to retrieve this information in any other way
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1080728528
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DubboClientContext.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import java.util.Objects;
+
+import io.micrometer.observation.transport.SenderContext;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+/**
+ * Provider context for RPC.
+ */
+public class DubboClientContext extends SenderContext<RpcContextAttachment> {
+
+ private final RpcContextAttachment rpcContextAttachment;
+
+ private final Invoker<?> invoker;
+
+ private final Invocation invocation;
+
+ public DubboClientContext(RpcContextAttachment rpcContextAttachment, Invoker<?> invoker, Invocation invocation) {
+ super((map, key, value) -> Objects.requireNonNull(map).setAttachment(key, value));
+ this.rpcContextAttachment = rpcContextAttachment;
+ this.invoker = invoker;
+ this.invocation = invocation;
+ setCarrier(rpcContextAttachment);
+ }
Review Comment:
```suggestion
public DubboClientContext(Invoker<?> invoker, Invocation invocation) {
super((map, key, value) -> Objects.requireNonNull(map).setAttachment(key, value));
this.invoker = invoker;
this.invocation = invocation;
setCarrier(invocation);
}
```
Directly set attachments, which can be send to remote, by `invocation.setAttachment`. `RpcContextAttachment` is design as a user-mode API, not for framework itself.
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
`NET_PEER_NAME` is design for provider side or consumer side?
If for provider side, use `org.apache.dubbo.rpc.RpcContext#getServiceContext.getRemoteAddress()` instead. `RpcContextAttachment` is design for attachment only. The `getRemotePort` here is for compatible purpose.
If for consumer side, it is not a good idea to track it because consumer can call for retry or boardcast. Thus, it is hard to fetch the actual remote address, but it is possible. :)
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
+
+ private ObservationRegistry observationRegistry = ObservationRegistry.NOOP;
+
+ private DubboClientObservationConvention clientObservationConvention = null;
+
+ @Override
+ public void setApplicationModel(ApplicationModel applicationModel) {
+ observationRegistry = applicationModel.getBeanFactory().getBean(ObservationRegistry.class);
+ clientObservationConvention = applicationModel.getBeanFactory().getBean(DubboClientObservationConvention.class);
+ }
Review Comment:
```suggestion
public ObservationSenderFilter(ApplicationModel applicationModel) {
observationRegistry = applicationModel.getBeanFactory().getBean(ObservationRegistry.class);
clientObservationConvention = applicationModel.getBeanFactory().getBean(DubboClientObservationConvention.class);
}
```
Use constructor instead is better, which can prevent NPE in initial.
BTW, as for test, you can use `ApplicationModel.defaultModel()` for test purpose. `defaultModel` is only for test or compatible purpose and should be prevented to being used for production code.
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationReceiverFilter.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.PROVIDER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the incoming message.
+ */
+@Activate(group = PROVIDER, order = -1)
+public class ObservationReceiverFilter implements Filter, BaseFilter.Listener, ScopeModelAware {
+
+ private ObservationRegistry observationRegistry = ObservationRegistry.NOOP;
+
+ private DubboServerObservationConvention serverObservationConvention = null;
+
+ @Override
+ public void setApplicationModel(ApplicationModel applicationModel) {
+ observationRegistry = applicationModel.getBeanFactory().getBean(ObservationRegistry.class);
+ serverObservationConvention = applicationModel.getBeanFactory().getBean(DubboServerObservationConvention.class);
+ }
+
+ @Override
+ public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
+ if (observationRegistry == null) {
+ return invoker.invoke(invocation);
+ }
+ RpcContextAttachment context = RpcContext.getServerAttachment();
+ DubboServerContext receiverContext = new DubboServerContext(context, invoker, invocation);
Review Comment:
Use invocation instead.
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationReceiverFilter.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.PROVIDER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the incoming message.
+ */
+@Activate(group = PROVIDER, order = -1)
+public class ObservationReceiverFilter implements Filter, BaseFilter.Listener, ScopeModelAware {
+
+ private ObservationRegistry observationRegistry = ObservationRegistry.NOOP;
+
+ private DubboServerObservationConvention serverObservationConvention = null;
+
+ @Override
+ public void setApplicationModel(ApplicationModel applicationModel) {
Review Comment:
The same
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
+ return appendNonNull(keyValues, NET_PEER_PORT, String.valueOf(port));
+ }
+
+ private KeyValues appendNonNull(KeyValues keyValues, KeyName keyName, @Nullable String value) {
+ if (value != null) {
+ return keyValues.and(keyName.withValue(value));
+ }
+ return keyValues;
+ }
+
+ String getContextualName(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ String method = StringUtils.hasText(rpcContextAttachment.getMethodName()) ? rpcContextAttachment.getMethodName() : invocation.getMethodName();
+ return serviceName + "/" + method;
+ }
Review Comment:
`Invocation` would be a higher priority. Can ignore `rpcContextAttachment` here.
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
+
+ private ObservationRegistry observationRegistry = ObservationRegistry.NOOP;
+
+ private DubboClientObservationConvention clientObservationConvention = null;
+
+ @Override
+ public void setApplicationModel(ApplicationModel applicationModel) {
+ observationRegistry = applicationModel.getBeanFactory().getBean(ObservationRegistry.class);
+ clientObservationConvention = applicationModel.getBeanFactory().getBean(DubboClientObservationConvention.class);
+ }
+
+ @Override
+ public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
+ if (observationRegistry == null) {
+ return invoker.invoke(invocation);
+ }
+ RpcContextAttachment context = RpcContext.getClientAttachment();
+ DubboClientContext senderContext = new DubboClientContext(context, invoker, invocation);
+ Observation observation = DubboObservation.CLIENT.observation(this.clientObservationConvention, DefaultDubboClientObservationConvention.INSTANCE, () -> senderContext, observationRegistry);
+ return observation.observe(() -> invoker.invoke(invocation));
+ }
+
+ @Override
+ public void onResponse(Result appResponse, Invoker<?> invoker, Invocation invocation) {
+
+ }
+
+ @Override
+ public void onError(Throwable t, Invoker<?> invoker, Invocation invocation) {
Review Comment:
Should we track if return errors?
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
+
+ private ObservationRegistry observationRegistry = ObservationRegistry.NOOP;
+
+ private DubboClientObservationConvention clientObservationConvention = null;
+
+ @Override
+ public void setApplicationModel(ApplicationModel applicationModel) {
+ observationRegistry = applicationModel.getBeanFactory().getBean(ObservationRegistry.class);
+ clientObservationConvention = applicationModel.getBeanFactory().getBean(DubboClientObservationConvention.class);
+ }
+
+ @Override
+ public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
+ if (observationRegistry == null) {
+ return invoker.invoke(invocation);
+ }
+ RpcContextAttachment context = RpcContext.getClientAttachment();
+ DubboClientContext senderContext = new DubboClientContext(context, invoker, invocation);
+ Observation observation = DubboObservation.CLIENT.observation(this.clientObservationConvention, DefaultDubboClientObservationConvention.INSTANCE, () -> senderContext, observationRegistry);
+ return observation.observe(() -> invoker.invoke(invocation));
Review Comment:
In async mode, `invoker.invoke` would return quickly in a short time and return a furure. If you want to get the actual return time, track in `onResponse` method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by "marcingrzejszczak (via GitHub)" <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1084035512
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/DefaultDubboClientObservationConvention.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+
+/**
+ * Default implementation of the {@link DubboClientObservationConvention}.
+ */
+public class DefaultDubboClientObservationConvention extends AbstractDefaultDubboObservationConvention implements DubboClientObservationConvention {
+ /**
+ * Singleton instance of {@link DefaultDubboClientObservationConvention}.
+ */
+ public static final DubboClientObservationConvention INSTANCE = new DefaultDubboClientObservationConvention();
+
+ @Override
+ public String getName() {
+ return "rpc.client.duration";
+ }
+
+ @Override
+ public KeyValues getLowCardinalityKeyValues(DubboClientContext context) {
+ KeyValues keyValues = super.getLowCardinalityKeyValues(context.getInvocation());
+ RpcContextAttachment rpcContextAttachment = context.getRpcContextAttachment();
+ URL url = context.getInvoker().getUrl();
Review Comment:
Shouldn't there be multiple observations there, one for each retry?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1066536270
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
Review Comment:
> I have a question, why does `ObservationSenderFilter` implements `ClusterFilter` not `Filter`. Are there any special considerations？
@marcingrzejszczak
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1337138095
Sure, if you have any questions I'll be more than happy to answer them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1073549865
##########
dubbo-metrics/dubbo-metrics-api/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
+observationsender=org.apache.dubbo.metrics.filter.observation.ObservationSenderFilter
Review Comment:
OK, ignore this first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1081084317
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
Good catch, that's for the client https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#http-client and that's mandatory to be set
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by "marcingrzejszczak (via GitHub)" <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1083914190
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/AbstractDefaultDubboObservationConvention.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.docs.KeyName;
+import io.micrometer.common.lang.Nullable;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_NAME;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.NET_PEER_PORT;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_METHOD;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SERVICE;
+import static org.apache.dubbo.metrics.filter.observation.DubboObservation.LowCardinalityKeyNames.RPC_SYSTEM;
+
+class AbstractDefaultDubboObservationConvention {
+ KeyValues getLowCardinalityKeyValues(Invocation invocation, RpcContextAttachment rpcContextAttachment) {
+ KeyValues keyValues = KeyValues.of(RPC_SYSTEM.withValue("apache_dubbo"));
+ String serviceName = StringUtils.hasText(invocation.getServiceName()) ? invocation.getServiceName() : readServiceName(invocation.getTargetServiceUniqueName());
+ keyValues = appendNonNull(keyValues, RPC_SERVICE, serviceName);
+ keyValues = appendNonNull(keyValues, RPC_METHOD, invocation.getMethodName());
+ keyValues = appendNonNull(keyValues, NET_PEER_NAME, rpcContextAttachment.getRemoteHostName());
+ if (rpcContextAttachment.getRemotePort() == 0) {
+ return keyValues;
+ }
+ int port = rpcContextAttachment.getRemotePort() != 0 ? rpcContextAttachment.getRemotePort() : rpcContextAttachment.getLocalPort();
Review Comment:
I don't know if I did this correctly - if I didn't then I need someone to tell me how to do it properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #11021: Added support for Micrometer Observation API
Posted by sonarcloud.
sonarcloud[bot] commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1400333944
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=11021)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11021&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11021&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list) [43.1% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list) [23.3% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11021&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by "marcingrzejszczak (via GitHub)" <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1401994500
> 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 :shrug: BTW you can find this here https://github.com/apache/dubbo/pull/11021/files#diff-dfd2e12c668d7d1cb35baf3117d32272b38a03e0bec26dbd21a324098d90783dR138-R145
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on pull request #11021: Added support for Micrometer Observation API
Posted by "marcingrzejszczak (via GitHub)" <gi...@apache.org>.
marcingrzejszczak commented on PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#issuecomment-1402151813
Fixed the broken test, sorry about that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] marcingrzejszczak commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
marcingrzejszczak commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1034660097
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ *
+ * @author Marcin Grzejszczak
+ */
+@Activate(group = CONSUMER, order = -1)
Review Comment:
They are both connected. When we're doing an observation we need to have access to the attachments so that we mutate those. We instrument only once and the handlers will do the attachment mutation on start and on stop of the observation. Unless I'm missing sth...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] conghuhu commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
conghuhu commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1066901359
##########
dubbo-metrics/dubbo-metrics-api/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
Review Comment:
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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] chickenlj commented on a diff in pull request #11021: Added support for Micrometer Observation API
Posted by GitBox <gi...@apache.org>.
chickenlj commented on code in PR #11021:
URL: https://github.com/apache/dubbo/pull/11021#discussion_r1066539422
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/filter/observation/ObservationSenderFilter.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.metrics.filter.observation;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.BaseFilter;
+import org.apache.dubbo.rpc.Filter;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.Result;
+import org.apache.dubbo.rpc.RpcContext;
+import org.apache.dubbo.rpc.RpcContextAttachment;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.filter.ClusterFilter;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ScopeModelAware;
+
+import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER;
+
+/**
+ * A {@link Filter} that creates an {@link Observation} around the outgoing message.
+ */
+@Activate(group = CONSUMER, order = -1)
+public class ObservationSenderFilter implements ClusterFilter, BaseFilter.Listener, ScopeModelAware {
Review Comment:
Above is how `ClusterFilter` and `Filter` work. Most of the time, we recommend implementing `ClusterFilter` for it generates fewer instances, especially when working in a large cluster with lots of instances. Only consider implementing `Filter` when you have customized needs for different instances.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org