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

[NOJIRA] Use the ParsePath function to gather the cluster name and run ID #248

Merged
merged 4 commits into from
Sep 11, 2024

Conversation

edznux-dd
Copy link
Contributor

This PR has been created to resolve #246

I believe some deeper work would be needed to refactor the config loading: it's difficult to know what's the source of truth for some value, and what's populated when.
For example the clustername is set:

  • As a flag
  • As a viper value
  • As part of the Dynamic config
  • As part of the Ingestor config
  • As part of the Collector config
  • As part of the filename of the dump

Since we are using a pointer to the config in most functions, I believe that consolidating the ClusterName and RunID to be part of the "root" config element would already solve most of the issues.
The question of "when is the data populated" could still be a bit confusing since some code path would populate it at different stage, but I don't see a simple solution for that here.

Minosity-VR
Minosity-VR previously approved these changes Sep 11, 2024
Copy link
Contributor

@Minosity-VR Minosity-VR left a comment

Choose a reason for hiding this comment

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

Tested locally, works on macos. Unless Ed wants to check for linux, lgtm!

pkg/cmd/dump.go Outdated Show resolved Hide resolved
Co-authored-by: Simon Maréchal <[email protected]>
@jt-dd jt-dd merged commit 3f4ca04 into main Sep 11, 2024
8 checks passed
@jt-dd jt-dd deleted the edouard/fix-cluster-name-propagation branch September 11, 2024 12:13
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.

[Help needed] File collector cluster name not provided
3 participants