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

Add config to set target database for metric query #450

Merged
merged 4 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion tembo-operator/Cargo.lock

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

2 changes: 1 addition & 1 deletion tembo-operator/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "controller"
description = "Tembo Operator for Postgres"
version = "0.29.0"
version = "0.29.1"
edition = "2021"
default-run = "controller"
license = "Apache-2.0"
Expand Down
2 changes: 1 addition & 1 deletion tembo-operator/src/app_service/ingress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ pub fn generate_ingress_tcp_routes(
let mut routes: Vec<IngressRouteTCPRoutes> = Vec::new();
for route in routings.iter() {
match route.ingress_path.clone() {
Some(path) => {
Some(_path) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is to fix a clippy warning where path isn't being used.

if !route.ingress_type.clone()?.eq(&IngressType::tcp) {
// Do not create IngressRouteTCPRoutes for non-TCP ingress type
debug!("Skipping IngressRouteTCPRoutes for non-TCP ingress type");
Expand Down
4 changes: 4 additions & 0 deletions tembo-operator/src/defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ pub fn default_postgres_exporter_image() -> String {
"quay.io/prometheuscommunity/postgres-exporter:v0.12.0".to_owned()
}

pub fn default_postgres_exporter_target_databases() -> Vec<String> {
vec!["postgres".to_owned()]
}

pub fn default_extensions() -> Vec<Extension> {
vec![]
}
Expand Down
9 changes: 9 additions & 0 deletions tembo-operator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ pub enum Error {
#[error("SerializationError: {0}")]
SerializationError(#[source] serde_json::Error),

#[error("SerializationError: {0}")]
YamlSerializationError(#[source] serde_yaml::Error),

#[error("Kube Error: {0}")]
KubeError(#[from] kube::Error),

Expand All @@ -71,3 +74,9 @@ impl Error {
format!("{self:?}").to_lowercase()
}
}

impl From<serde_yaml::Error> for Error {
fn from(err: serde_yaml::Error) -> Self {
Error::YamlSerializationError(err)
}
}
26 changes: 22 additions & 4 deletions tembo-operator/src/postgres_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ pub struct Metrics {
/// - total_messages:
/// description: Total number of messages that have passed into the queue.
/// usage: GAUGE
/// target_databases:
/// - "postgres"
/// ```
#[derive(Clone, Debug, JsonSchema, PartialEq, Serialize, Deserialize)]
pub struct QueryItem {
Expand All @@ -131,6 +133,16 @@ pub struct QueryItem {
/// description: the metric's description
/// metrics_mapping: the optional column mapping when usage is set to MAPPEDMETRIC
pub metrics: Vec<Metrics>,

/// The default database can always be overridden for a given user-defined
/// metric, by specifying a list of one or more databases in the target_databases
/// option.
///
/// See: [https://cloudnative-pg.io/documentation/1.20/monitoring/#example-of-a-user-defined-metric-running-on-multiple-databases](https://cloudnative-pg.io/documentation/1.20/monitoring/#example-of-a-user-defined-metric-running-on-multiple-databases)
///
/// **Default:** `["postgres"]`
#[serde(default = "defaults::default_postgres_exporter_target_databases")]
pub target_databases: Vec<String>,
}

#[derive(Clone, Debug, JsonSchema, PartialEq, Serialize, Deserialize)]
Expand Down Expand Up @@ -185,7 +197,7 @@ pub async fn reconcile_metrics_configmap(cdb: &CoreDB, client: Client, ns: &str)
// directly and not through the reconcile function.
match cdb.spec.metrics.clone().and_then(|m| m.queries) {
Some(queries) => {
let qdata = serde_yaml::to_string(&queries).unwrap();
let qdata = serde_yaml::to_string(&queries)?;
let d: BTreeMap<String, String> = BTreeMap::from([(QUERIES.to_string(), qdata)]);
apply_configmap(
client.clone(),
Expand All @@ -196,7 +208,7 @@ pub async fn reconcile_metrics_configmap(cdb: &CoreDB, client: Client, ns: &str)
.await?
}
None => {
debug!("No queries specified in CoreDB spec");
debug!("No queries specified in CoreDB spec {}", coredb_name);
}
}
Ok(())
Expand All @@ -223,7 +235,8 @@ mod tests {
"description": "Time at which postmaster started"
}
}
]
],
"target_databases": ["postgres"]
},
"extensions": {
"query": "select count(*) as num_ext from pg_available_extensions",
Expand All @@ -235,7 +248,8 @@ mod tests {
"description": "Num extensions"
}
}
]
],
"target_databases": ["postgres"]
}
}
);
Expand Down Expand Up @@ -286,13 +300,17 @@ mod tests {
- num_ext:
usage: GAUGE
description: Num extensions
target_databases:
- postgres
pg_postmaster:
query: SELECT pg_postmaster_start_time as start_time_seconds from pg_postmaster_start_time()
master: true
metrics:
- start_time_seconds:
usage: GAUGE
description: Time at which postmaster started
target_databases:
- postgres
"#;
// formmatted correctly as yaml (for configmap)
assert_eq!(yaml, data);
Expand Down
2 changes: 2 additions & 0 deletions tembo-operator/src/stacks/templates/message_queue.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ postgres_metrics:
- total_messages:
usage: GAUGE
description: Total number of messages that have passed into the queue.
target_databases:
- "postgres"
postgres_config_engine: mq
postgres_config:
- name: shared_preload_libraries
Expand Down
4 changes: 3 additions & 1 deletion tembo-operator/yaml/sample-message-queue.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ spec:
image: quay.io/prometheuscommunity/postgres-exporter:v0.12.0
queries:
pgmq:
query: select queue_name, queue_length, oldest_msg_age_sec, newest_msg_age_sec, total_messages from public.pgmq_metrics_all()
query: select queue_name, queue_length, oldest_msg_age_sec, newest_msg_age_sec, total_messages from pgmq.metrics_all()
master: true
metrics:
- queue_name:
Expand All @@ -73,6 +73,8 @@ spec:
- total_messages:
description: Total number of messages that have passed into the queue.
usage: GAUGE
target_databases:
Copy link
Member

Choose a reason for hiding this comment

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

Do we also need to add this to the Stack definition at src/stacks/templates?

Copy link
Collaborator Author

@nhudson nhudson Dec 21, 2023

Choose a reason for hiding this comment

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

No it will add it automatically when reconciling the ConfigMap. Though, it wouldn't hurt to add it to the stack definition

- "postgres"
runtime_config:
- name: shared_buffers
value: "1024MB"
Expand Down
Loading