Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert does not correctly handle converting EMADynamicSamplerConfig.AdjustmentInterval #766

Closed
TylerHelmuth opened this issue Jul 6, 2023 · 1 comment · Fixed by #768
Labels
tool: convert type: bug Something isn't working
Milestone

Comments

@TylerHelmuth
Copy link
Contributor

Versions

  • Refinery: 2.0.0

Steps to reproduce

input:

rules:
  Sampler: DeterministicSampler
  SampleRate: 1
  test: # E&S kibble rules (hence lowercase)
    Sampler: RulesBasedSampler
    rule:
      - name: test
        condition:
          - field: service.name
            operator: =
            value: test
        sampler:
          EMADynamicSampler:
            Sampler: EMADynamicSampler
            AdjustmentInterval: 60
      - name: default rule
        SampleRate: 1

expected output:

rules:
    RulesVersion: 2
    Samplers:
        __default__:
            DeterministicSampler:
                SampleRate: 1
        test:
            RulesBasedSampler:
                Rules:
                    - Name: test
                      Conditions:
                        - Field: service.name
                          Operator: =
                          Value: test
                      Sampler:
                        EMADynamicSampler:
                            AdjustmentInterval: 1m0s
                    - Name: default rule
                      SampleRate: 1

actual output:

panic: readV1RulesIntoV2Sampler unable to unmarshal config: json: cannot unmarshal number into Go struct field EMADynamicSamplerConfig.rule.sampler.emadynamicsampler.adjustmentinterval of type config.Duration

goroutine 1 [running]:
main.convertRulesToNewConfig(0x105213f20?)
        /Users/tylerhelmuth/projects/honeycomb/refinery/tools/convert/ruleconvert.go:165 +0x25c
main.ConvertHelm(0x1400000e348, {0x1052503c8?, 0x14000010060})
        /Users/tylerhelmuth/projects/honeycomb/refinery/tools/convert/main.go:346 +0x55c
main.main()
        /Users/tylerhelmuth/projects/honeycomb/refinery/tools/convert/main.go:216 +0x93c

Process finished with the exit code 2

Additional context

It appears the the rules convert logic doesn't handle the situation where the old value was an int and the new value is a duration.

@TylerHelmuth TylerHelmuth added type: bug Something isn't working tool: convert labels Jul 6, 2023
@TylerHelmuth
Copy link
Contributor Author

I appears the issue is that the converter does not recursively transfrom the RulesBaseSampler's sampler.

@TylerHelmuth TylerHelmuth added this to the v2.0.1 milestone Jul 7, 2023
TylerHelmuth added a commit that referenced this issue Jul 10, 2023
…sed samples (#768)

## Which problem is this PR solving?

- closes #766

## Short description of the changes

- hacks away at an issue where the convert tool couldnt handle the
[]Rules in the rules based sampler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool: convert type: bug Something isn't working
Projects
None yet
1 participant