Skip to content

Fix disable statement not working#6687

Merged
wu-sheng merged 1 commit intomasterfrom
fix-disable
Apr 5, 2021
Merged

Fix disable statement not working#6687
wu-sheng merged 1 commit intomasterfrom
fix-disable

Conversation

@wu-sheng
Copy link
Member

@wu-sheng wu-sheng commented Apr 5, 2021

Fix

  • Add a unit test to verify that the fix works.
  • Explain briefly why the bug exists and how to fix it.

This bug was introduced through @arugal 's #4748. As we changed the loading priority and sequence of the OAL scripts, it did make sure disable statements could be found before loading hardcoded metric/record/...

I add a new disable.oal, and give it the highest priority. All disable statements have to be put in there.

@wu-sheng wu-sheng added bug Something isn't working and you are sure it's a bug! backend OAP backend related. labels Apr 5, 2021
@wu-sheng wu-sheng added this to the 8.5.0 milestone Apr 5, 2021
@wu-sheng wu-sheng requested review from kezhenxu94 and rainbend April 5, 2021 09:36
@wu-sheng
Copy link
Member Author

wu-sheng commented Apr 5, 2021

@nisiyong Please take a look.

@codecov
Copy link

codecov bot commented Apr 5, 2021

Codecov Report

Merging #6687 (1a7094f) into master (d7beac5) will increase coverage by 11.72%.
The diff coverage is 100.00%.

❗ Current head 1a7094f differs from pull request most recent head 2e2083b. Consider uploading reports for the commit 2e2083b to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             master    #6687       +/-   ##
=============================================
+ Coverage     47.25%   58.97%   +11.72%     
- Complexity     3391     4214      +823     
=============================================
  Files          1792      999      -793     
  Lines         38412    25066    -13346     
  Branches       4234     2447     -1787     
=============================================
- Hits          18152    14783     -3369     
+ Misses        19265     9006    -10259     
- Partials        995     1277      +282     
Impacted Files Coverage Δ Complexity Δ
...skywalking/oap/server/core/CoreModuleProvider.java 81.87% <100.00%> (+3.25%) 10.00 <0.00> (ø)
...lking/oap/server/core/oal/rt/DisableOALDefine.java 100.00% <100.00%> (ø) 2.00 <2.00> (?)
...g/oap/server/core/cluster/ClusterHealthStatus.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...p/server/core/analysis/metrics/MinLongMetrics.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...er/exporter/provider/grpc/GRPCExporterSetting.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...king/oap/server/configuration/api/ConfigTable.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...skywalking/oap/server/receiver/envoy/als/Role.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...r/receiver/envoy/als/k8s/ServiceNameFormatter.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...ng/oap/meter/analyzer/dsl/tagOpt/K8sRetagType.java 0.00% <0.00%> (-92.86%) 0.00% <0.00%> (ø%)
...t/status/HierarchyMatchExceptionCheckStrategy.java 9.09% <0.00%> (-90.91%) 1.00% <0.00%> (ø%)
... and 1341 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7beac5...2e2083b. Read the comment docs.

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@rainbend rainbend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wu-sheng wu-sheng merged commit 2b85ba1 into master Apr 5, 2021
@wu-sheng wu-sheng deleted the fix-disable branch April 5, 2021 12:11
@nisiyong
Copy link
Contributor

nisiyong commented Apr 5, 2021

@nisiyong Please take a look.

👍I try it tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend OAP backend related. bug Something isn't working and you are sure it's a bug!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAL disable statement doesn't work

4 participants