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.

Sign up for GitHub

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

WIP: feat(metrics): add actix-web metrics exporter #793

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

WIP: feat(metrics): add actix-web metrics exporter #793

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
96f32e4
feat(metrics): add actix-web metrics exporter
lefuturiste Aug 1, 2023
10bef25
cargo update
nyurik Jul 17, 2023
47b03f6
Demo: Migrate to yarn (#770)
nyurik Jul 17, 2023
8c53c27
chore(deps): Bump clap from 4.3.14 to 4.3.17 (#776)
dependabot[bot] Jul 20, 2023
71041ee
update lock
nyurik Jul 21, 2023
10a12d8
chore(deps): Bump clap from 4.3.17 to 4.3.19 (#779)
dependabot[bot] Jul 24, 2023
a261402
chore(deps): Bump async-trait from 0.1.71 to 0.1.72 (#781)
dependabot[bot] Jul 24, 2023
ea750ed
chore(deps): Bump thiserror from 1.0.43 to 1.0.44 (#780)
dependabot[bot] Jul 24, 2023
558f2f1
update lock
nyurik Jul 24, 2023
76488b6
make licensing terms more visible
nyurik Jul 25, 2023
0e588a4
Add basic support for systemd (#761)
sharkAndshark Jul 26, 2023
386e827
chore(deps): Bump serde_json from 1.0.103 to 1.0.104 (#783)
dependabot[bot] Jul 27, 2023
c502bf5
chore(deps): Bump subst from 0.2.2 to 0.2.3 (#784)
dependabot[bot] Jul 27, 2023
a0302e0
fix readme links
nyurik Jul 27, 2023
f2d21a1
Add ability to copy to existing mbtiles files (#778)
upsicleclown Jul 27, 2023
dc535e5
Create primary keys on metadata and tiles tables when copying with fo…
upsicleclown Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ actix-cors = "0.6"
actix-http = "3"
actix-rt = "2"
actix-web = "4"
actix-web-prom = "0.6.0"
anyhow = "1.0"
async-trait = "0.1"
brotli = "3"
Expand Down
1 change: 1 addition & 0 deletions martin/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ spreet.workspace = true
subst.workspace = true
thiserror.workspace = true
tilejson.workspace = true
actix-web-prom.workspace = true
tokio = { workspace = true, features = ["io-std"] }

# Optional dependencies for ssl support
Expand Down
18 changes: 18 additions & 0 deletions martin/src/srv/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ use crate::srv::config::{SrvConfig, KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT
use crate::utils::{decode_brotli, decode_gzip, encode_brotli, encode_gzip};
use crate::Error::BindingError;

use actix_web_prom::PrometheusMetricsBuilder;
use std::collections::HashMap;

/// List of keywords that cannot be used as source IDs. Some of these are reserved for future use.
/// Reserved keywords must never end in a "dot number" (e.g. ".1")
pub const RESERVED_KEYWORDS: &[&str] = &[
Expand Down Expand Up @@ -257,6 +260,9 @@ fn merge_tilejson(sources: Vec<&dyn Source>, tiles_url: String) -> TileJSON {
result
}

// TODO: Change here, to add an attribute to tell actix-web-prom that we want to keep the
// cardinality of {source_ids}

#[route("/{source_ids}/{z}/{x}/{y}", method = "GET", method = "HEAD")]
async fn get_tile(
req: HttpRequest,
Expand Down Expand Up @@ -407,6 +413,17 @@ pub fn router(cfg: &mut web::ServiceConfig) {

/// Create a new initialized Actix `App` instance together with the listening address.
pub fn new_server(config: SrvConfig, all_sources: AllSources) -> crate::Result<(Server, String)> {
// metrics setup
let labels = HashMap::new();

// labels.insert("label_foo".to_string(), "value_barbarbbabbmiles".to_string());
let prometheus = PrometheusMetricsBuilder::new("martin")
.endpoint("/metrics")
Copy link
Member

Choose a reason for hiding this comment

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

if we start handling /metrics, we would have to declare this as a reserved keyword (we might be missing docs in a martin book for this). One workaround perhaps would be to add everything "magical" to the /_/metrics style prefixes because _ is not a valid source ID i think.

martin/martin/src/srv/server.rs

Lines 32 to 37 in a2acd17

/// List of keywords that cannot be used as source IDs. Some of these are reserved for future use.
/// Reserved keywords must never end in a "dot number" (e.g. ".1")
pub const RESERVED_KEYWORDS: &[&str] = &[
"catalog", "config", "font", "health", "help", "index", "manifest", "refresh", "reload",
"sprite", "status",
];

Copy link
Author

Choose a reason for hiding this comment

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

if we start handling /metrics, we would have to declare this as a reserved keyword (we might be missing docs in a martin book for this). One workaround perhaps would be to add everything "magical" to the /_/metrics style prefixes because _ is not a valid source ID i think.

martin/martin/src/srv/server.rs

Lines 32 to 37 in a2acd17

/// List of keywords that cannot be used as source IDs. Some of these are reserved for future use.
/// Reserved keywords must never end in a "dot number" (e.g. ".1")
pub const RESERVED_KEYWORDS: &[&str] = &[
"catalog", "config", "font", "health", "help", "index", "manifest", "refresh", "reload",
"sprite", "status",
];

Right, as you wish, I'm fine with both solutions (the reserved keyword and the /_/metrics workaround). We just have to keep in mind that /metrics is the standard path and the default, so the second solution would mean a little bit more config on the scraper side.

Copy link
Member

Choose a reason for hiding this comment

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

good point about conventions (actually, didn't we talk about it elsewhere?), i'm ok to add /metrics to reserved then

.const_labels(labels)
.build()
.unwrap();
// end of metrics setup

let keep_alive = Duration::from_secs(config.keep_alive.unwrap_or(KEEP_ALIVE_DEFAULT));
let worker_processes = config.worker_processes.unwrap_or_else(num_cpus::get);
let listen_addresses = config
Expand All @@ -424,6 +441,7 @@ pub fn new_server(config: SrvConfig, all_sources: AllSources) -> crate::Result<(
.wrap(cors_middleware)
.wrap(middleware::NormalizePath::new(TrailingSlash::MergeOnly))
.wrap(middleware::Logger::default())
.wrap(prometheus.clone())
.configure(router)
})
.bind(listen_addresses.clone())
Expand Down