search

tikv / rust-prometheus

Prometheus instrumentation library for Rust applications
Apache License 2.0
1.04k stars 182 forks source link

feat: export inner MetricVecBuilder structs #490

Open MrCroxx opened 1 year ago

MrCroxx commented 1 year ago

Signed-off-by: MrCroxx mrcroxx@outlook.com

This PR exports MetricVecBuilder types, with which users can easily build customized tools.

MrCroxx commented 1 year ago

In risingwave#9756 I built self-managed metrics generic types with the inner builder types. IMO exporting the inner builder helps a lot. Is It okay to export them to all user or is there any better way to achieve it? Thanks a lot. 🙏

TennyZhuang commented 1 year ago

You should also bump the MSRV in the CI file.

MrCroxx commented 1 year ago

Seems a new version of indirect dependency requires the new rust version. 🤔

MrCroxx commented 1 year ago

@nickelc PTAL 🙏 If this PR is reasonable. Thanks a lot.

MrCroxx commented 1 year ago

@lucab PTAL

nickelc commented 1 year ago

The changes look fine.

Seems a new version of indirect dependency requires the new rust version. thinking

The dev dependency rayon 1.7 requires Rust 1.59 and the indirect dependency security-framework v2.9.0 now requires Rust 1.60.

$ cargo tree --target all -i security-framework --features push
security-framework v2.9.0
└── native-tls v0.2.11
    ├── hyper-tls v0.5.0
    │   └── reqwest v0.11.17
    │       └── prometheus v0.13.3 (/projects/various/rust-prometheus)
    ├── reqwest v0.11.17 (*)
    └── tokio-native-tls v0.3.1
        ├── hyper-tls v0.5.0 (*)
        └── reqwest v0.11.17 (*)
MrCroxx commented 1 year ago

@nickelc Thanks for help. Should I pin the dependency to the version that meets the MSRV(1.57) requires or update MSRV to a higher version?

lucab commented 1 year ago

@MrCroxx thanks for the patch! Let's bump the MSRV, 1.60 sounds like a good target. Could you please do that in a separate PR? I'll review and merge that one, so that we can first unblock CI and then get to the rest of changes later.

MrCroxx commented 1 year ago

@nickelc Could you please do that in a separate PR? I'll review and merge that one, so that we can first unblock CI and then get to the rest of changes later.

Sure~

Done with #491 .

MrCroxx commented 1 year ago

Besides, would you like to publish a new version of this lib after this PR is merged?

MrCroxx commented 1 year ago

@nickelc Sorry to bother you again. Can this PR be merged?

MrCroxx commented 10 months ago

@lucab Hi, any updates? Can this PR be merged?