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

change(log): Report compiler version and Zebra features when starting Zebra #6606

Merged
merged 9 commits into from
May 14, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented May 4, 2023

Motivation

We want to report the compiler version when Zebra panics, and when it starts up. This helps us diagnose some bugs.

As part of this change, I updated the vergen version, because it changes how the compiler version works.

Close #5422
Close #6421

Specifications

API reference:
https://docs.rs/vergen/latest/vergen/index.html#usage

Complex Code or Requirements

This might break some of Zebra's existing version-handling code, which expects a semver-compliant version number.

It might also cause issues in PR #6601, which manually parses the version number.

Solution

  • Add the compiler version and compiler date (for nightly compilers) to logs and panics

Related changes:

  • Add a list of enabled Zebra features to logs and panics
  • Upgrade to vergen version 8 (ticket build(deps): bump vergen from 7.5.1 to 8.0.0 #6421)
    • Rewrite the vergen build script
    • Remove outdated disabled reproducible build code, vergen now has an env var to activate it
    • Replace the cargo profile with debug checks and optimization level
    • Change some environmental variable names
    • Turn vergen build errors into compiler warnings if .git doesn't exist

Testing

See my manual testing here:
#6606 (comment)

Review

This is a routine change.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@teor2345 teor2345 added A-dependencies Area: Dependency file updates C-enhancement Category: This is an improvement C-cleanup Category: This is a cleanup P-Medium ⚡ A-diagnostics Area: Diagnosing issues or monitoring performance labels May 4, 2023
@teor2345 teor2345 self-assigned this May 4, 2023
@github-actions github-actions bot added C-feature Category: New features C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels May 4, 2023
@teor2345
Copy link
Contributor Author

teor2345 commented May 4, 2023

We have 3 different alternatives for checking the git metadata, I'm just doing some builds to check which one is the fastest to build. (It's probably the C library dependency, unfortunately!)

@teor2345
Copy link
Contributor Author

teor2345 commented May 4, 2023

Using gitoxide adds 80 new crate dependencies to Zebra's build (596 -> 675), which increases our security risk a bit.

So that's another potential issue.

@teor2345
Copy link
Contributor Author

teor2345 commented May 4, 2023

main branch:
real 2m18.323s
user 47m53.429s
sys 2m22.332s

This PR with gitoxide dependency:
real 2m22.013s
user 48m26.231s
sys 2m21.810s

33 seconds more user compile time

This PR with git2 dependency:
real 2m19.301s
user 47m48.147s
sys 2m21.963s

5 seconds less user compile time

@teor2345 teor2345 marked this pull request as ready for review May 4, 2023 01:53
@teor2345 teor2345 requested a review from a team as a code owner May 4, 2023 01:53
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team May 4, 2023 01:53
@teor2345
Copy link
Contributor Author

teor2345 commented May 4, 2023

Manual testing:

Before this PR, Zebra's startup logs have:

$ zebrad
2023-05-04T00:05:38.784742Z  INFO zebrad::application: Diagnostic Metadata:
version: 1.0.0-rc.7+30.g937d704
Zcash network: Mainnet
state version: 25
branch: report-compiler-version
git commit: 937d704
commit timestamp: 2023-05-03T23:14:22Z
target triple: x86_64-unknown-linux-gnu
build profile: release
2023-05-04T00:05:38.784744Z  INFO zebrad::application: loaded zebrad config ...

After this PR, they have:

$ zebrad
2023-05-04T01:59:18.728120Z  INFO zebrad::application: Diagnostic Metadata:
version: 1.0.0-rc.7-37+gedf51d6.dirty
Zcash network: Mainnet
state version: 25
features: default,getblocktemplate_rpcs,metrics_exporter_prometheus,prometheus,release_max_level_info
branch: report-compiler-version
git commit: 0623ba6
commit timestamp: 1980-01-01T00:00:00.000000000Z
target triple: x86_64-unknown-linux-gnu
rust compiler: 1.71.0-nightly
rust release date: 2023-05-02
optimization level: 3
debug checks: true
2023-05-04T01:59:18.728122Z  INFO zebrad::application: loaded zebrad config ...

The fields removed by vergen 8 are:

build profile: release

The new fields are:

features: default,getblocktemplate_rpcs,metrics_exporter_prometheus,prometheus,release_max_level_info
branch: report-compiler-version
commit timestamp: 1980-01-01T00:00:00.000000000Z
rust compiler: 1.71.0-nightly
rust release date: 2023-05-02
optimization level: 3
debug checks: true

The changed fields are:

version: 1.0.0-rc.7+30.g937d704.modified
to
version: 1.0.0-rc.7-37+gedf51d6.dirty

But in most builds they are unchanged:

version: 1.0.0-rc.7-37+gedf51d6
or
version: 1.0.0-rc.7

@teor2345 teor2345 removed C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG C-feature Category: New features labels May 4, 2023
@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Merging #6606 (40e98ad) into main (0190882) will increase coverage by 0.02%.
The diff coverage is 46.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6606      +/-   ##
==========================================
+ Coverage   77.87%   77.90%   +0.02%     
==========================================
  Files         309      309              
  Lines       40665    40669       +4     
==========================================
+ Hits        31669    31684      +15     
+ Misses       8996     8985      -11     

@teor2345
Copy link
Contributor Author

teor2345 commented May 4, 2023

I'm going to bump this down to low priority because it doesn't need to go in the upcoming release.

@teor2345 teor2345 force-pushed the report-compiler-version branch from 5607bbf to 8247d38 Compare May 7, 2023 21:32
@github-actions github-actions bot added the C-feature Category: New features label May 7, 2023
@teor2345 teor2345 added P-Medium ⚡ and removed C-cleanup Category: This is a cleanup C-feature Category: New features P-Low ❄️ labels May 7, 2023
@teor2345 teor2345 force-pushed the report-compiler-version branch from 8247d38 to 40e98ad Compare May 11, 2023 23:30
@github-actions github-actions bot added C-feature Category: New features C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels May 11, 2023
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

mergify bot added a commit that referenced this pull request May 14, 2023
@mergify mergify bot merged commit df949a2 into main May 14, 2023
@mergify mergify bot deleted the report-compiler-version branch May 14, 2023 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency file updates A-diagnostics Area: Diagnosing issues or monitoring performance C-enhancement Category: This is an improvement C-feature Category: New features C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build(deps): bump vergen from 7.5.1 to 8.0.0 Add the Rust compiler version to the bug report metadata
2 participants