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

feat: [v2] Use the new configuration system #690

Merged
merged 8 commits into from
May 26, 2023
Merged

Conversation

kentquirk
Copy link
Contributor

@kentquirk kentquirk commented May 24, 2023

This converts refinery to use the new configuration model that we've been moving towards

This is an enormous PR because there really wasn't a good way to change to a new configuration model slowly; it affected a lot of support code and tests.

Key elements include:

  • The main configuration was reorganized into a collection of structs, each of which has appropriate struct tags for validation and defaults.
  • All the configuration support code now uses the new configuration structs. Some of the function names have been changed, and more will be changed in this or later PRs.
  • The Config interface changed to support this, which means that the mock config also changed.
  • Rules have been restructured so that the rules file can be read without pre-reading it to understand its structure. This also enables it to be validated normally.
  • All duration-related values are now specified as Duration objects and must be specified with units (like 3m30s or 100ms).
  • Code supporting the legacy sharding system was removed. This required updating a couple of tests that had been designed around deterministic traceIDs.
  • SampleCache no longer supports the legacy trace caching mechanism. Its code was deleted and tests updated.
  • The "resize" cache management strategy was removed. Its code was deleted and tests updated.
  • Tests requiring configuration were changed to generate YAML from a map rather than trying to embed YAML into the test scripts. This shortened them a lot and makes them easier to maintain.
  • Along the way, several issues were uncovered in the configuration as it had been redefined and so those items were updated and the template regenerated.

There is still work to do before this can be released, but it's passing tests and I can go back to smaller PRs after this.

This closes a few of the planned issues for 2.0:
Closes #667
Closes #655
Closes #581

@kentquirk kentquirk marked this pull request as ready for review May 24, 2023 20:40
@kentquirk kentquirk requested a review from a team as a code owner May 24, 2023 20:40
@kentquirk kentquirk added version: bump major A PR that breaks backwards compatibility. breaking-change Prefer 'version: bump major', but use this for breaking changes that don't bump major. labels May 24, 2023
@kentquirk kentquirk added this to the v2.0 milestone May 24, 2023
@kentquirk kentquirk changed the title feat: Use the new configuration system [WIP] feat: [v2] Use the new configuration system May 24, 2023
Copy link
Contributor

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

I expected to see the new that new throughput sampler set as the default sampler in the rules. Are we still doing that and should it be part of this PR?

config/file_config.go Show resolved Hide resolved
config/file_config.go Show resolved Hide resolved
config/level.go Outdated Show resolved Hide resolved
@kentquirk
Copy link
Contributor Author

The new samplers will come in a separate PR.

tools/convert/configData.yaml Outdated Show resolved Hide resolved
tools/convert/configData.yaml Outdated Show resolved Hide resolved
@kentquirk kentquirk requested a review from TylerHelmuth May 26, 2023 03:11
@kentquirk kentquirk merged commit 3c518fc into main May 26, 2023
@kentquirk kentquirk deleted the kent.use_new_config branch May 26, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Prefer 'version: bump major', but use this for breaking changes that don't bump major. version: bump major A PR that breaks backwards compatibility.
Projects
None yet
2 participants