From 50674d698741d100be8b7566bc9666d1f7fe4a31 Mon Sep 17 00:00:00 2001 From: kenstott <128912107+kenstott@users.noreply.github.com> Date: Mon, 26 Aug 2024 13:13:32 -0400 Subject: [PATCH] Improved tracing and debug logs. --- crates/calcite-schema/src/aggregates.rs | 2 +- crates/calcite-schema/src/collections.rs | 1 + crates/calcite-schema/src/comparators.rs | 6 ++--- crates/calcite-schema/src/configuration.rs | 8 +++++-- crates/calcite-schema/src/jvm.rs | 4 ++-- crates/calcite-schema/src/models.rs | 2 +- crates/calcite-schema/src/scalars.rs | 5 ++-- crates/calcite-schema/src/schema.rs | 2 +- crates/calcite-schema/src/version.rs | 3 +++ crates/calcite-schema/src/version5.rs | 3 +++ crates/connectors/ndc-calcite/src/calcite.rs | 4 ++-- .../ndc-calcite/src/capabilities.rs | 2 ++ crates/connectors/ndc-calcite/src/query.rs | 24 +++++++++---------- crates/connectors/ndc-calcite/src/sql.rs | 4 ++-- crates/values/src/is_running_in_container.rs | 3 ++- 15 files changed, 43 insertions(+), 30 deletions(-) diff --git a/crates/calcite-schema/src/aggregates.rs b/crates/calcite-schema/src/aggregates.rs index 3b1c38f..466ab68 100644 --- a/crates/calcite-schema/src/aggregates.rs +++ b/crates/calcite-schema/src/aggregates.rs @@ -25,7 +25,7 @@ use ndc_models::{AggregateFunctionDefinition, AggregateFunctionName, Type}; /// # Returns /// /// A `BTreeMap` containing aggregate function definitions for `sum`, `max`, `avg`, and `min`. -#[tracing::instrument(skip(underlying_type))] +#[tracing::instrument(skip(underlying_type), level=Level::INFO)] pub fn numeric_aggregates( underlying_type: &str, ) -> BTreeMap { diff --git a/crates/calcite-schema/src/collections.rs b/crates/calcite-schema/src/collections.rs index fe73b0e..1b45412 100644 --- a/crates/calcite-schema/src/collections.rs +++ b/crates/calcite-schema/src/collections.rs @@ -28,6 +28,7 @@ use crate::calcite::{ColumnMetadata, TableMetadata}; /// /// An inner Result can also be returned, which contains an error indicating an issue with the input data. // ANCHOR: collections +#[tracing::instrument(skip(data_models, scalar_types), level=Level::INFO)] pub fn collections( data_models: &HashMap, scalar_types: &BTreeMap, diff --git a/crates/calcite-schema/src/comparators.rs b/crates/calcite-schema/src/comparators.rs index b7b7bfa..faa3780 100644 --- a/crates/calcite-schema/src/comparators.rs +++ b/crates/calcite-schema/src/comparators.rs @@ -3,8 +3,8 @@ //! You can create new comparison functions here. //! use std::collections::BTreeMap; - use ndc_models::{ComparisonOperatorDefinition, ComparisonOperatorName, Type, TypeName}; +use tracing::{Level}; /// Generate string comparison operators based on the underlying type. /// @@ -23,7 +23,7 @@ use ndc_models::{ComparisonOperatorDefinition, ComparisonOperatorName, Type, Typ /// /// ``` // ANCHOR: string_comparators -#[tracing::instrument(skip(numeric_comparison_operators))] +#[tracing::instrument(skip(numeric_comparison_operators), level=Level::INFO)] pub fn string_comparators( numeric_comparison_operators: &BTreeMap, ) -> BTreeMap { @@ -81,7 +81,7 @@ pub fn string_comparators( /// } /// ``` // ANCHOR: numeric_comparators -#[tracing::instrument(skip(underlying))] +#[tracing::instrument(skip(underlying), level=Level::INFO)] pub fn numeric_comparators(underlying: String) -> BTreeMap { let numeric_comparison_operators = BTreeMap::from_iter([ ("_eq".into(), ComparisonOperatorDefinition::Equal), diff --git a/crates/calcite-schema/src/configuration.rs b/crates/calcite-schema/src/configuration.rs index 1631212..3d51c7e 100644 --- a/crates/calcite-schema/src/configuration.rs +++ b/crates/calcite-schema/src/configuration.rs @@ -1,7 +1,6 @@ //! Configuration for the connector. use std::path::{Path, PathBuf}; - use crate::environment::Environment; use crate::error::{ MakeRuntimeConfigurationError, ParseConfigurationError, @@ -11,8 +10,9 @@ use crate::version5; use crate::version::VersionTag; use schemars::{gen::SchemaSettings, schema::RootSchema}; use crate::version5::CalciteRefSingleton; +use tracing::{Level}; - +#[tracing::instrument(skip(), level=Level::INFO)] pub fn generate_latest_schema() -> RootSchema { SchemaSettings::openapi3() .into_generator() @@ -38,9 +38,11 @@ pub enum ParsedConfiguration { type Configuration = ParsedConfiguration; impl ParsedConfiguration { + #[tracing::instrument(skip(), level=Level::INFO)] pub fn initial() -> Self { ParsedConfiguration::Version5(version5::ParsedConfiguration::empty()) } + #[tracing::instrument(skip_all, level=Level::INFO)] pub fn version(&self) -> VersionTag { match self { ParsedConfiguration::Version5(_) => VersionTag::Version5, @@ -89,6 +91,7 @@ pub async fn parse_configuration( /// /// Each concrete supported version implementation is responsible for interpretation its format /// into the runtime configuration. +#[tracing::instrument(skip(parsed_config,_environment), level=Level::INFO)] pub fn make_runtime_configuration( parsed_config: ParsedConfiguration, _environment: impl Environment, @@ -112,6 +115,7 @@ pub async fn write_parsed_configuration( /// /// This is part of the configuration crate API to enable users to upgrade their configurations /// mechanically, using the ndc-postgres cli, when new versions are released. +#[tracing::instrument(skip(parsed_config), level=Level::INFO)] pub fn upgrade_to_latest_version(parsed_config: ParsedConfiguration) -> ParsedConfiguration { match parsed_config { ParsedConfiguration::Version5(_) => parsed_config, diff --git a/crates/calcite-schema/src/jvm.rs b/crates/calcite-schema/src/jvm.rs index c66041e..99076b5 100644 --- a/crates/calcite-schema/src/jvm.rs +++ b/crates/calcite-schema/src/jvm.rs @@ -33,7 +33,7 @@ static CONFIG: OnceCell> = OnceCell::new(); /// } /// ``` // ANCHOR: get_jvm -#[tracing::instrument(skip())] +#[tracing::instrument(skip(), level=Level::INFO)] pub fn get_jvm() -> &'static Mutex { { let jvm = JVM.get().expect("JVM is not set up."); @@ -69,7 +69,7 @@ pub fn get_jvm() -> &'static Mutex { /// init_jvm(&config); /// ``` // ANCHOR: init_jvm -#[tracing::instrument(skip(calcite_configuration))] +#[tracing::instrument(skip(calcite_configuration), level=Level::INFO)] pub fn init_jvm(calcite_configuration: &ParsedConfiguration) { let configuration = match calcite_configuration { ParsedConfiguration::Version5(c) => c diff --git a/crates/calcite-schema/src/models.rs b/crates/calcite-schema/src/models.rs index 34bbfe7..570272a 100644 --- a/crates/calcite-schema/src/models.rs +++ b/crates/calcite-schema/src/models.rs @@ -17,7 +17,7 @@ use crate::jvm::get_jvm; /// A `HashMap` containing the retrieved models. The outer `HashMap` maps model names /// to inner `HashMap`s, where each inner `HashMap` represents a model with its properties. -#[tracing::instrument(skip(calcite_ref))] +#[tracing::instrument(skip(calcite_ref), level=Level::INFO)] pub fn get_models(calcite_ref: &GlobalRef) -> HashMap { let map = { let jvm = get_jvm().lock().unwrap(); diff --git a/crates/calcite-schema/src/scalars.rs b/crates/calcite-schema/src/scalars.rs index 39fbf0d..c85d468 100644 --- a/crates/calcite-schema/src/scalars.rs +++ b/crates/calcite-schema/src/scalars.rs @@ -3,10 +3,9 @@ //! Make additional changes to scalar definitions here. //! use std::collections::BTreeMap; - use ndc_models::{ComparisonOperatorDefinition, ScalarType, ScalarTypeName, TypeRepresentation}; - use crate::{aggregates, comparators}; +use tracing::{Level}; /// Retrieves a mapping of scalar types with their respective properties. /// @@ -67,7 +66,7 @@ use crate::{aggregates, comparators}; /// } /// ``` // ANCHOR: scalars -#[tracing::instrument(skip())] +#[tracing::instrument(skip(), level=Level::INFO)] pub fn scalars() -> BTreeMap { let string_comparison_operators = comparators::string_comparators(&comparators::numeric_comparators("VARCHAR".into())); diff --git a/crates/calcite-schema/src/schema.rs b/crates/calcite-schema/src/schema.rs index ff7a4e9..75db05d 100644 --- a/crates/calcite-schema/src/schema.rs +++ b/crates/calcite-schema/src/schema.rs @@ -60,7 +60,7 @@ use crate::version5::ParsedConfiguration; /// } /// ``` // ANCHOR: get_schema -#[tracing::instrument(skip(configuration, calcite_ref))] +#[tracing::instrument(skip(configuration, calcite_ref), level=Level::INFO)] pub fn get_schema(configuration: &ParsedConfiguration, calcite_ref: GlobalRef) -> Result> { let data_models = get_models(&calcite_ref); let scalar_types = scalars::scalars(); diff --git a/crates/calcite-schema/src/version.rs b/crates/calcite-schema/src/version.rs index e908a80..e1503ac 100644 --- a/crates/calcite-schema/src/version.rs +++ b/crates/calcite-schema/src/version.rs @@ -1,3 +1,5 @@ +use tracing::{Level}; + #[derive(Debug, Copy, Clone)] pub enum VersionTag { Version3, @@ -6,6 +8,7 @@ pub enum VersionTag { } /// Emit deprecation warning text if the version is deprecated. +#[tracing::instrument(skip(version), level=Level::INFO)] pub fn deprecated_config_warning(version: VersionTag) -> Option { match version { VersionTag::Version3 => Some( diff --git a/crates/calcite-schema/src/version5.rs b/crates/calcite-schema/src/version5.rs index a222fd2..84e2db7 100644 --- a/crates/calcite-schema/src/version5.rs +++ b/crates/calcite-schema/src/version5.rs @@ -33,6 +33,7 @@ impl CalciteRefSingleton { } } + #[tracing::instrument(skip(self, args), level=Level::INFO)] pub fn initialize(&self, args: &crate::configuration::ParsedConfiguration) -> Result<(), &'static str> { match args { crate::configuration::ParsedConfiguration::Version5(config) => { @@ -102,6 +103,7 @@ impl ParsedConfiguration { } } +#[tracing::instrument(skip(configuration, calcite_query, env), level=Level::INFO)] pub fn create_calcite_connection<'a, 'b>( configuration: &'a ParsedConfiguration, calcite_query: &'b JObject, @@ -130,6 +132,7 @@ pub fn create_calcite_connection<'a, 'b>( } } +#[tracing::instrument(skip(configuration, env), level=Level::INFO)] pub fn create_calcite_query_engine<'a>( configuration: &'a ParsedConfiguration, mut env: JNIEnv<'a> diff --git a/crates/connectors/ndc-calcite/src/calcite.rs b/crates/connectors/ndc-calcite/src/calcite.rs index a91ebb3..af9ffc2 100644 --- a/crates/connectors/ndc-calcite/src/calcite.rs +++ b/crates/connectors/ndc-calcite/src/calcite.rs @@ -52,7 +52,7 @@ pub type Row = IndexMap; /// return instance; /// } /// ``` -#[tracing::instrument(skip(configuration, env))] +#[tracing::instrument(skip(configuration, env), level=Level::INFO)] pub fn create_calcite_query_engine<'a>(configuration: &'a ParsedConfiguration, env: &'a mut JNIEnv<'a>) -> JObject<'a> { let class = env.find_class("org/kenstott/CalciteQuery").unwrap(); let instance = env.new_object(class, "()V", &[]).unwrap(); @@ -115,7 +115,7 @@ fn parse_to_row(data: Vec) -> Vec { /// } /// ``` // ANCHOR: calcite_query -#[tracing::instrument(skip(config,calcite_reference,sql_query,query_metadata))] +#[tracing::instrument(skip(config,calcite_reference,sql_query,query_metadata), level=Level::INFO)] pub fn calcite_query( config: &ParsedConfiguration, calcite_reference: GlobalRef, diff --git a/crates/connectors/ndc-calcite/src/capabilities.rs b/crates/connectors/ndc-calcite/src/capabilities.rs index 558bd3d..88f4ffa 100644 --- a/crates/connectors/ndc-calcite/src/capabilities.rs +++ b/crates/connectors/ndc-calcite/src/capabilities.rs @@ -10,6 +10,7 @@ use ndc_models::{ Capabilities, CapabilitiesResponse, LeafCapability, MutationCapabilities, NestedFieldCapabilities, QueryCapabilities, RelationshipCapabilities, }; +use tracing::{Level}; /// Calculates the capabilities of the Calcite system. /// @@ -31,6 +32,7 @@ use ndc_models::{ /// /// - `CapabilitiesResponse`: A struct representing the capabilities of the Calcite system. // ANCHOR: calcite_capabilities +#[tracing::instrument(skip(), level=Level::INFO)] pub fn calcite_capabilities() -> CapabilitiesResponse { CapabilitiesResponse { version: "0.1.4".into(), diff --git a/crates/connectors/ndc-calcite/src/query.rs b/crates/connectors/ndc-calcite/src/query.rs index b01e4b6..a1728f7 100644 --- a/crates/connectors/ndc-calcite/src/query.rs +++ b/crates/connectors/ndc-calcite/src/query.rs @@ -11,7 +11,7 @@ use ndc_models::{ArgumentName, CollectionName, ComparisonOperatorName, Compariso use ndc_sdk::connector::QueryError; use ndc_sdk::models; use serde_json::{Number, Value}; -use tracing::debug; +use tracing::{debug, Level}; use crate::calcite::{calcite_query, Row}; use ndc_calcite_schema::version5::ParsedConfiguration; @@ -95,7 +95,7 @@ pub struct QueryComponents { /// } /// } /// ``` -#[tracing::instrument(skip(query_params))] +#[tracing::instrument(skip(query_params), level=Level::INFO)] pub fn orchestrate_query( query_params: QueryParams ) -> Result { @@ -132,7 +132,7 @@ pub fn orchestrate_query( return Ok(models::RowSet { aggregates: parsed_aggregates, rows: rows_data }); } -#[tracing::instrument(skip(rows_data, sub_relationship))] +#[tracing::instrument(skip(rows_data, sub_relationship), level=Level::DEBUG)] fn generate_value_from_rows(rows_data: &Vec, sub_relationship: &Relationship) -> Result { let relationship_value: Value = rows_data.into_iter().map(|row| { let mut row_values: Vec = Vec::new(); @@ -152,7 +152,7 @@ fn generate_value_from_rows(rows_data: &Vec, sub_relationship: &Relationshi Ok(relationship_value) } -#[tracing::instrument(skip(sub_relationship))] +#[tracing::instrument(skip(sub_relationship), level=Level::DEBUG)] fn parse_relationship(sub_relationship: &Relationship) -> Result<(Vec<(FieldName, FieldName)>, Vec<&FieldName>, RelationshipType), QueryError> { let pks: Vec<(FieldName, FieldName)> = sub_relationship.column_mapping .iter() @@ -166,7 +166,7 @@ fn parse_relationship(sub_relationship: &Relationship) -> Result<(Vec<(FieldName Ok((pks, fks, relationship_type)) } -#[tracing::instrument(skip(params, query_components))] +#[tracing::instrument(skip(params, query_components), level=Level::DEBUG)] fn process_rows(params: QueryParams, query_components: &QueryComponents) -> Result>, QueryError> { if let Some(phrase) = &query_components.select { if phrase.is_empty() && !query_components.final_aggregates.is_empty() { @@ -192,7 +192,7 @@ fn process_rows(params: QueryParams, query_components: &QueryComponents) -> Resu } -#[tracing::instrument(skip(params, query_components))] +#[tracing::instrument(skip(params, query_components), level=Level::DEBUG)] fn process_aggregates(params: QueryParams, query_components: &QueryComponents) -> Result>, QueryError> { if let Some(phrase) = &query_components.aggregates { if phrase.is_empty() { @@ -232,7 +232,7 @@ fn process_aggregates(params: QueryParams, query_components: &QueryComponents) - } } -#[tracing::instrument(skip(pks, value))] +#[tracing::instrument(skip(pks, value), level=Level::DEBUG)] fn generate_predicate(pks: &Vec<(FieldName, FieldName)>, value: Value) -> Result { let (_, name) = pks[0].clone(); Ok(Expression::BinaryComparisonOperator { @@ -246,7 +246,7 @@ fn generate_predicate(pks: &Vec<(FieldName, FieldName)>, value: Value) -> Result }) } -#[tracing::instrument(skip(query, predicate, pks))] +#[tracing::instrument(skip(query, predicate, pks), level=Level::DEBUG)] fn revise_query(query: Box, predicate: Expression, pks: &Vec<(FieldName, FieldName)>) -> Result, QueryError> { let mut revised_query = query.clone(); revised_query.predicate = Some(predicate); @@ -267,7 +267,7 @@ fn revise_query(query: Box, predicate: Expression, pks: &Vec<(FieldName, Ok(revised_query) } -#[tracing::instrument(skip(params, arguments, sub_relationship, revised_query))] +#[tracing::instrument(skip(params, arguments, sub_relationship, revised_query), level=Level::DEBUG)] fn execute_query(params: QueryParams, arguments: &BTreeMap, sub_relationship: &Relationship, revised_query: &Query) -> Result, QueryError> { let fk_rows = orchestrate_query(QueryParams { config: params.config, @@ -282,7 +282,7 @@ fn execute_query(params: QueryParams, arguments: &BTreeMap, field_name: &FieldName, fk_rows: &Vec, pks: &Vec<(FieldName, FieldName)>, fks: &Vec<&FieldName>) -> Result>, QueryError> { let modified_rows: Vec = rows.clone().into_iter().map(|mut row| { debug!("fk_rows: {:?}, row: {:?}, field_name: {:?}", serde_json::to_string_pretty(&fk_rows), serde_json::to_string_pretty(&row), field_name); @@ -315,7 +315,7 @@ fn process_object_relationship(rows: Vec, field_name: &FieldName, fk_rows: Ok(Some(modified_rows)) } -#[tracing::instrument(skip(rows, field_name, fk_rows, pks, fks, query))] +#[tracing::instrument(skip(rows, field_name, fk_rows, pks, fks, query), level=Level::DEBUG)] fn process_array_relationship(rows: Option>, field_name: &FieldName, fk_rows: &Vec, pks: &Vec<(FieldName, FieldName)>, fks: &Vec<&FieldName>, query: &Query) -> Result>, QueryError> { let modified_rows: Vec = rows.clone().unwrap().into_iter().map(|mut row| { debug!("fk_rows: {:?}, row: {:?}, field_name: {:?}", serde_json::to_string_pretty(&fk_rows), serde_json::to_string_pretty(&row), field_name); @@ -349,7 +349,7 @@ fn process_array_relationship(rows: Option>, field_name: &FieldName, fk Ok(Some(modified_rows)) } -#[tracing::instrument(skip(child_rows, rowset, value))] +#[tracing::instrument(skip(child_rows, rowset, value), level=Level::DEBUG)] fn process_child_rows(child_rows: &Vec<&Row>, mut rowset: serde_json::map::Map, value: &mut RowFieldValue) -> Result<(), QueryError> { rowset.insert("aggregates".to_string(), Value::Null); if !child_rows.is_empty() { diff --git a/crates/connectors/ndc-calcite/src/sql.rs b/crates/connectors/ndc-calcite/src/sql.rs index 242a247..e3f0744 100644 --- a/crates/connectors/ndc-calcite/src/sql.rs +++ b/crates/connectors/ndc-calcite/src/sql.rs @@ -453,7 +453,7 @@ fn create_qualified_table_name(table_metadata: &TableMetadata) -> String { path.join(".") } -#[tracing::instrument(skip(configuration,_arguments, select, order_by, pagination, where_clause, join_clause),level=Level::DEBUG)] +#[tracing::instrument(skip(configuration,_arguments, select, order_by, pagination, where_clause, join_clause),level=Level::INFO)] pub fn query_collection( configuration: &ParsedConfiguration, collection_name: &CollectionName, @@ -545,7 +545,7 @@ pub fn query_collection( arguments, query, variables -))] +), level=Level::INFO)] pub fn parse_query<'a>(configuration: &'a ParsedConfiguration, collection: &'a CollectionName, collection_relationships: &'a BTreeMap, arguments: &'a BTreeMap, query: &'a Query, variables: &'a BTreeMap) -> Result { let mut argument_values = BTreeMap::new(); for (argument_name, argument_value) in arguments { diff --git a/crates/values/src/is_running_in_container.rs b/crates/values/src/is_running_in_container.rs index 532dc22..cef4ee8 100644 --- a/crates/values/src/is_running_in_container.rs +++ b/crates/values/src/is_running_in_container.rs @@ -1,5 +1,6 @@ use std::env; use std::path::Path; +use tracing::{Level}; /// Checks if the code is running inside a container. /// @@ -21,7 +22,7 @@ use std::path::Path; /// # Returns /// /// Returns `true` if the code is running inside a container, `false` otherwise. -#[tracing::instrument(skip())] +#[tracing::instrument(skip(), level=Level::INFO)] pub fn is_running_in_container() -> bool { Path::new("/.dockerenv").exists() || env::var("KUBERNETES_SERVICE_HOST").is_ok() } \ No newline at end of file