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

pd: 📦 add a tracing event with cargo version #3952

Merged
merged 1 commit into from
Mar 5, 2024
Merged

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Mar 5, 2024

this adds a small tracing event including the version of pd that is running, before proceeding to handle the CLI command.

this should help operators identify when they are running a stale version of pd.

this adds a small tracing event including the version of pd that is
running, before proceeding to handle the CLI command.

this should help operators identify when they are running a stale
version of pd.
@cratelyn cratelyn added the A-telemetry Area: Metrics, logging, and other observability-related features label Mar 5, 2024
@cratelyn cratelyn added this to the Sprint 1 milestone Mar 5, 2024
@cratelyn cratelyn self-assigned this Mar 5, 2024
@cratelyn cratelyn merged commit adb8e59 into main Mar 5, 2024
6 checks passed
@cratelyn cratelyn deleted the kate/log-pd-version branch March 5, 2024 21:40
@hdevalence
Copy link
Member

Maybe I'm missing something — this doesn't actually do what we want, right? That environment variable is set by Cargo when running Cargo, not when running pd...

@cratelyn
Copy link
Contributor Author

cratelyn commented Mar 5, 2024

Maybe I'm missing something — this doesn't actually do what we want, right? That environment variable is set by Cargo when running Cargo, not when running pd...

CARGO_PKG_VERSION — The full version of your package.

https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates

to my understanding, this would log versions like 0.68.1 or 0.68.2 etc., based on the version of the workspace/crate. this is set at compile-time like you describe, but that'd resolve to effectively let version = "1.2.3"; in the resulting binary.

in the interest of disambiguating whether a stale pd is running, that seems like at least a small step in the right direction. i'm happy to revert this though. what do you think @hdevalence?

@hdevalence
Copy link
Member

My read of that page is that it's set by cargo, not embedded in the binary. But an easy way to test would be to run the binary ./target/release/pd and see.

@cratelyn
Copy link
Contributor Author

cratelyn commented Mar 5, 2024

My read of that page is that it's set by cargo, not embedded in the binary. But an easy way to test would be to run the binary ./target/release/pd and see.

i confirmed this works properly with a small test program:

; eza -T --ignore-glob target
.
├── Cargo.lock
├── Cargo.toml
└── src
   └── main.rs
; bat Cargo.toml | grep version
version = "0.1.0"
; ./target/debug/env-playground
0.1.0
; sed -i -e 's/0.1.0/1.2.3/' Cargo.toml
; cargo build --quiet
; ./target/debug/env-playground
1.2.3

@hdevalence
Copy link
Member

Ah great! This is much simpler than I'd expected then, thanks for getting it merged in!

conorsch pushed a commit that referenced this pull request Mar 6, 2024
this adds a small tracing event including the version of pd that is
running, before proceeding to handle the CLI command.

this should help operators identify when they are running a stale
version of pd.

(cherry picked from commit adb8e59)
conorsch pushed a commit that referenced this pull request Mar 6, 2024
this adds a small tracing event including the version of pd that is
running, before proceeding to handle the CLI command.

this should help operators identify when they are running a stale
version of pd.

(cherry picked from commit adb8e59)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-telemetry Area: Metrics, logging, and other observability-related features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants