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

Telemetry config generated on Linux platforms does not work on non-Linux platforms on 4.0 #84

Open
hgmich opened this issue Oct 29, 2024 · 2 comments

Comments

@hgmich
Copy link

hgmich commented Oct 29, 2024

If you use foundations' built-in telemetry settings to generate your application config, the memory profiler options are only included if the package is built for Linux. This didn't previously cause issues, but since 4.0 made it an error to include unknown config fields, running the service on non-Linux OSes such as macOS cause these configs to error.

Given that common development workflows may involve a mix of running an application on Docker and natively on e.g. macOS, it would probably be better if these options were just ignored when not applicable; potentially with a warning message indicating they have no effect.

@inikulin
Copy link
Collaborator

That's a valid concern. For now you can restore the old behaviour via a feature toggle: https://github.com/cloudflare/foundations/blob/main/foundations/Cargo.toml#L60C1-L60C40
However, it's not ergonomic as it was before, because now you would need to turn off default features and manually enabled those that you need.

However, I think we should teach the settings macro to recognise the OS feature gates and don't complain about unused fields in such cases. Contributions are welcome!

@hgmich
Copy link
Author

hgmich commented Nov 21, 2024

My concern there would be interactions; the simple case of "block of options that only exist on some platforms" is easy to cover, but the behaviour would be more complex in other cases which can be expressed#[cfg()] attributes, such as overlapping field definitions for specific platforms.

Since I'm opinionated, my feelings are the better resolutions are one of these options:

  • Make strict validation of config options an opt-in feature, rather than an opt-out one.
  • Keep strict validation as is, but discourage using #[cfg()] in settings structs in documentation (and discontinue their current use). This feels consistent with other documentation recommendations (e.g. using an enabled field rather than making sections Optional).

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

No branches or pull requests

2 participants