search

alibaba / opentelemetry-go-auto-instrumentation

OpenTelemetry Compile-Time Instrumentation for Golang
Apache License 2.0
235 stars 31 forks source link

feat: coexist custom rules and default rules #185

Closed y1yang0 closed 1 week ago

y1yang0 commented 1 week ago

related to #184

RuleJsonFiles is the name of the rule file. It is used to tell instrument tool where to find the instrument rules. Multiple rules are separated by comma. e.g. -rule=rule1.json,rule2.json. By default, new rules are appended to default rules, i.e. -rule=rule1.json,rule2.json is exactly equivalent to -rule=default.json,rule1.json,rule2.json. But if you do want to replace the default rules, you can add a "+" prefix to the rule file name, e.g. -rule=+rule1.json,rule2.json. In this case, the default rules will be replaced by rule1.json, and then rule2.json will be appended to the rules.

codecov-commenter commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.58%. Comparing base (45c1afe) to head (3d0abca). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #185 +/- ## ========================================== - Coverage 83.78% 83.58% -0.20% ========================================== Files 19 19 Lines 771 780 +9 ========================================== + Hits 646 652 +6 - Misses 100 102 +2 - Partials 25 26 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

y1yang0 commented 1 week ago

@123liuziming Please take a look at this, it allows both myconf.json and default.json to coexist when user specifies -rule=myconf.json.

123liuziming commented 1 week ago

What is the expected behaviour if both default rule and custom rule enhance the same method?

y1yang0 commented 1 week ago

What is the expected behaviour if both default rule and custom rule enhance the same method?

Both of them are applied to the same method, regardless of whether they are located in a single JSON file or in two separate files

123liuziming commented 1 week ago

LGTM, I will try the extension demo in this branch and merge the PR after that.

y1yang0 commented 1 week ago

LGTM, I will try the extension demo in this branch and merge the PR after that.

One unrelated topic: I think we should at least add a test for these demos to ensure they all compile successfully, otherwise they are very fragile and prone to breaking.

123liuziming commented 1 week ago

LGTM, I will try the extension demo in this branch and merge the PR after that.

One unrelated topic: I think we should at least add a test for these demos to ensure they all compile successfully, otherwise they are very fragile and prone to breaking.

I will add a CI for this

y1yang0 commented 1 week ago

LGTM, I will try the extension demo in this branch and merge the PR after that.

One unrelated topic: I think we should at least add a test for these demos to ensure they all compile successfully, otherwise they are very fragile and prone to breaking.

I will add a CI for this

Thanks for bearing with me!

123liuziming commented 1 week ago

LGTM