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

Don't use reserved version key in logs #1542

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

RebeccaMahany
Copy link
Contributor

The logshipper adds a version key for current launcher version to logs, so using version keys elsewhere means we miss out on that logshipper data.

directionless
directionless previously approved these changes Jan 11, 2024
James-Pickett
James-Pickett previously approved these changes Jan 11, 2024
@James-Pickett
Copy link
Contributor

alternatively, we could update our current "version" keyword to "launcher_version"

@RebeccaMahany
Copy link
Contributor Author

@James-Pickett I'm fine either way -- do you have a preference? I didn't know if maybe the ingest server or something in Cloud Log relied on version -- but after looking into it, it doesn't seem like it does.

@James-Pickett
Copy link
Contributor

@James-Pickett I'm fine either way -- do you have a preference? I didn't know if maybe the ingest server or something in Cloud Log relied on version -- but after looking into it, it doesn't seem like it does.

feel like using version again in a log message is a possibility and we would need to lint for it or something, but if we switch to launcher_version we kind of future proof ourselves from making same error in the future

@RebeccaMahany RebeccaMahany added this pull request to the merge queue Jan 11, 2024
Merged via the queue into kolide:main with commit fd5742e Jan 11, 2024
25 checks passed
@RebeccaMahany RebeccaMahany deleted the becca/version-logs branch January 11, 2024 18:27
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