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

validator-new.version does not update when the identity is changed #34389

Closed
diman-io opened this issue Dec 10, 2023 · 4 comments
Closed

validator-new.version does not update when the identity is changed #34389

diman-io opened this issue Dec 10, 2023 · 4 comments
Labels
community Community contribution good first issue Good for newcomers

Comments

@diman-io
Copy link
Contributor

Problem

After updating the validator identity using solana-validator set-identity, the validator-new.version is not sent for the new identity.

Proposed Solution

Send it.

@diman-io diman-io added community Community contribution good first issue Good for newcomers labels Dec 10, 2023
@gystemd
Copy link

gystemd commented Dec 18, 2023

Hi, I would like to give a try for this issue. First, if I understood correctly, I should send the validator-new.version metric to the be saved in a InfluxDB. Is this correct?
I've tried to do this by placing the following line of code:

solana_metrics::datapoint_info!(
           "validator-new",
           ("version", solana_version::version!(), String),
);

here:


However, when I check the logs, this metric does not seem to appear by searching validator-new.
The script I did run for testing the change is multinode-demo/bootstrap-validator.sh, and I added the following
line:
$program --ledger "$ledger_dir" set-identity "$identity"

here, inside the loop:


I'm not sure if I'm on the right track and if I'm mistaken something in the configuration of the scripts/code.

@diman-io
Copy link
Contributor Author

Hi, thank you for diving into this issue!

I've tried to do this by...

Yes, that's correct, but make sure to check how it's sent in Validator::new(). You can find it by searching for "validator-new" in the entire codebase; it's the only instance.

here:

I think it would be better to place it in the AdminRpc implementation rather than in the CLI wrapper. Just follow the code you've tagged downwards, and you'll find the place where the key pair is updated for gossip. I was thinking that we could put our update there, right after the gossip update.

Feel free to ask me if you need further clarification or assistance!

@gystemd
Copy link

gystemd commented Dec 19, 2023

I think it would be better to place it in the AdminRpc implementation rather than in the CLI wrapper.

Indeed, it makes sense. Putting the datapoint_info! statement inside the AdminRpc implementation now produces the metric:
image

but make sure to check how it's sent in Validator::new()

Yes, that's what I've done in the first place. I just copied that statement and left only the "version" attribute as requested by the issue.
Thank you for the hints!

@kubanemil
Copy link

@diman-io I saw your conversation with @steviez in an attached PR, seems the status or description of the issue should be updated.

@diman-io diman-io reopened this May 23, 2024
@diman-io diman-io closed this as not planned Won't fix, can't repro, duplicate, stale May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants