You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/11/19 11:47:38 UTC
[GitHub] [skywalking] kezhenxu94 opened a new pull request #8157: Add filter mechanism in MAL core and fix some bugs
kezhenxu94 opened a new pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157
Also fixed some minor bugs
* Fix concurrency bug in MAL `increase`-related calculation.
* Fix a null pointer bug when building `SampleFamily`.
<!--
β οΈ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ====
### Fix <bug description or the bug issue number or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [ ] Explain briefly why the bug exists and how to fix it.
==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ==== -->
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes https://github.com/apache/skywalking/issues/8145.
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/CHANGES.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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wankai123 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753153239
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/vm.yaml
##########
@@ -28,6 +28,7 @@
# "-P6H3M" -- parses as "-6 hours and -3 minutes"
# "-P-6H+3M" -- parses as "+6 hours and -3 minutes"
# </pre>
+filter: "{ tags -> tags.job_name == 'vm-monitoring' }"
Review comment:
I found `oap.yaml` has the same metrics with `istio-controlplane.yaml`, not `vm`
```
process_cpu_seconds_total
```
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#issuecomment-974058091
`mal.md` should be updated about filter expression.
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wankai123 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753194971
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/k8s-cluster.yaml
##########
@@ -31,8 +31,6 @@
expSuffix: tag({tags -> tags.cluster = 'k8s-cluster::' + tags.cluster}).service(['cluster'])
Review comment:
we can add `kubernetes-cadvisor` and `kube-state-metrics` cause example used these name
https://github.com/apache/skywalking/blob/master/docs/en/setup/backend/otel-collector-config.yaml#L33
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753195036
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/k8s-cluster.yaml
##########
@@ -31,8 +31,6 @@
expSuffix: tag({tags -> tags.cluster = 'k8s-cluster::' + tags.cluster}).service(['cluster'])
Review comment:
Isn't k8s monitoring using OTEL job(s) to forward data?
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#issuecomment-974082316
filter is added to all MAL files now
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753196941
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/k8s-cluster.yaml
##########
@@ -31,8 +31,6 @@
expSuffix: tag({tags -> tags.cluster = 'k8s-cluster::' + tags.cluster}).service(['cluster'])
Review comment:
> we can add `kubernetes-cadvisor` and `kube-state-metrics` cause example used these name
> https://github.com/apache/skywalking/blob/master/docs/en/setup/backend/otel-collector-config.yaml#L33
If I have to add a filter, the names we use in the example are first choice for sure, my question is, should we add those if there is no obvious conflicts at the moment?
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753182879
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/vm.yaml
##########
@@ -28,6 +28,7 @@
# "-P6H3M" -- parses as "-6 hours and -3 minutes"
# "-P-6H+3M" -- parses as "+6 hours and -3 minutes"
# </pre>
+filter: "{ tags -> tags.job_name == 'vm-monitoring' }"
Review comment:
I think we should recommend(if not require) to add filter for all oc-receiver MAL. In the future, OpenTelemetry native metric format(just GA) and meter receiver should recommend this setup 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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753178900
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/k8s-cluster.yaml
##########
@@ -31,8 +31,6 @@
expSuffix: tag({tags -> tags.cluster = 'k8s-cluster::' + tags.cluster}).service(['cluster'])
Review comment:
> Should we add a filter for k8s too?
I'm not sure... on the one hand, adding a default filter requires the users to give the same job name when they set up OpenTelemetry job, otherwise there is no data and confused for users, on the other hand, add a filter is absolutely right to narrow down the metrics that need to be processed in this file
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753205492
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/k8s-cluster.yaml
##########
@@ -31,8 +31,6 @@
expSuffix: tag({tags -> tags.cluster = 'k8s-cluster::' + tags.cluster}).service(['cluster'])
Review comment:
I would recommend yes. Such as using a `Istio*` to filter all Istio MAL scripts, makes sense always.
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753172741
##########
File path: docs/en/setup/backend/backend-telemetry.md
##########
@@ -128,7 +128,7 @@ Set this up following these steps:
```
2. Set up OpenTelemetry Collector and config a scrape job:
``` yaml
-- job_name: 'skywalking'
+- job_name: 'skywalking-so11y' # make sure to use this in the so11y.yaml to filter only so11y metrics
Review comment:
Updated
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753223804
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/k8s-cluster.yaml
##########
@@ -31,8 +31,6 @@
expSuffix: tag({tags -> tags.cluster = 'k8s-cluster::' + tags.cluster}).service(['cluster'])
Review comment:
> I would recommend yes. Such as using a `Istio*` to filter all Istio MAL scripts, makes sense always.
Not sure what you mean. The names of jobs for istio doesnβt necessarily always include `istio*`, the job name is totally human written, it can be arbitrary
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753174476
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/vm.yaml
##########
@@ -28,6 +28,7 @@
# "-P6H3M" -- parses as "-6 hours and -3 minutes"
# "-P-6H+3M" -- parses as "+6 hours and -3 minutes"
# </pre>
+filter: "{ tags -> tags.job_name == 'vm-monitoring' }"
Review comment:
> I found `oap.yaml` has the same metrics with `istio-controlplane.yaml`, not `vm`
>
> ```
> process_cpu_seconds_total
> ```
There are metrics name conflicts in `vm` and `so11y` for sure, you cannot only compare our configuration files, the key is what metrics actually OpenTelemetry sends. FYI the conflict between `so11y` and `vm` is `instance_cpu_percentage` from what I can tell.
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753174884
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/k8s-cluster.yaml
##########
@@ -31,8 +31,6 @@
expSuffix: tag({tags -> tags.cluster = 'k8s-cluster::' + tags.cluster}).service(['cluster'])
Review comment:
Should we add a filter for k8s 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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wankai123 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753163054
##########
File path: docs/en/setup/backend/backend-telemetry.md
##########
@@ -128,7 +128,7 @@ Set this up following these steps:
```
2. Set up OpenTelemetry Collector and config a scrape job:
``` yaml
-- job_name: 'skywalking'
+- job_name: 'skywalking-so11y' # make sure to use this in the so11y.yaml to filter only so11y metrics
Review comment:
the example file need sync too :)
https://github.com/apache/skywalking/blob/master/docs/en/setup/backend/otel-collector-oap.yaml#L33
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a change in pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wankai123 commented on a change in pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157#discussion_r753190500
##########
File path: oap-server/server-starter/src/main/resources/otel-oc-rules/istio-controlplane.yaml
##########
@@ -28,6 +28,7 @@
# "-P6H3M" -- parses as "-6 hours and -3 minutes"
# "-P-6H+3M" -- parses as "+6 hours and -3 minutes"
# </pre>
+filter: "{ tags -> tags.job_name in ['kubernetes-pods', 'kubernetes-cadvisor', 'kube-state-metrics' ] }" # The OpenTelemetry job name
Review comment:
i think isito control-plane only required `kubernetes-pods` @hanahmily
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng merged pull request #8157: Add filter mechanism in MAL core and fix some bugs
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #8157:
URL: https://github.com/apache/skywalking/pull/8157
--
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@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org