Skip to content

Conversation

@SrikarMannepalli
Copy link
Contributor

@SrikarMannepalli SrikarMannepalli commented Sep 5, 2022

Description

This PR adds the rule type field for exclude span rules to distinguish between system exclude span rules and user exclude span rules. Depends on hypertrace/config-service#133.

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@SrikarMannepalli SrikarMannepalli requested a review from a team as a code owner September 5, 2022 04:12
@github-actions
Copy link

github-actions bot commented Sep 5, 2022

Test Results

24 tests  ±0   24 ✔️ ±0   15s ⏱️ -2s
11 suites ±0     0 💤 ±0 
11 files   ±0     0 ±0 

Results for commit 9a71868. ± Comparison against base commit d30509b.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Sep 5, 2022

Codecov Report

Merging #154 (9a71868) into main (d30509b) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main     #154   +/-   ##
=========================================
  Coverage     22.80%   22.80%           
  Complexity       75       75           
=========================================
  Files            65       65           
  Lines          1741     1741           
  Branches         53       53           
=========================================
  Hits            397      397           
  Misses         1335     1335           
  Partials          9        9           
Flag Coverage Δ
unit 22.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

RuleType convertExcludeSpanRuleRuleType(ExcludeSpanRuleRuleType ruleType) {
// TODO: remove this check after making this field non-nullable
if (ruleType == null) {
return RuleType.RULE_TYPE_UNSPECIFIED;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right. Updated it now.

case RULE_TYPE_SYSTEM:
case RULE_TYPE_UNSPECIFIED: // required to cater for the older user configs(as they didn't
// have a rule type field)
return ExcludeSpanRuleRuleType.SYSTEM;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right😅, updated it now.

@SrikarMannepalli SrikarMannepalli merged commit 0830119 into main Sep 5, 2022
@SrikarMannepalli SrikarMannepalli deleted the update-exclude-span-rule-schema branch September 5, 2022 12:29
skjindal93 pushed a commit that referenced this pull request Jun 21, 2024
Co-authored-by: aaron-steinfeld <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants