Skip to content

Commit

Permalink
Improved tracing and debug logs.
Browse files Browse the repository at this point in the history
  • Loading branch information
kenstott committed Aug 26, 2024
1 parent 81651dc commit 50674d6
Show file tree
Hide file tree
Showing 15 changed files with 43 additions and 30 deletions.
2 changes: 1 addition & 1 deletion crates/calcite-schema/src/aggregates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<AggregateFunctionName, AggregateFunctionDefinition> {
Expand Down
1 change: 1 addition & 0 deletions crates/calcite-schema/src/collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CollectionName, TableMetadata>,
scalar_types: &BTreeMap<ScalarTypeName, ScalarType>,
Expand Down
6 changes: 3 additions & 3 deletions crates/calcite-schema/src/comparators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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<ComparisonOperatorName, ComparisonOperatorDefinition>,
) -> BTreeMap<ComparisonOperatorName, ComparisonOperatorDefinition> {
Expand Down Expand Up @@ -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<ComparisonOperatorName, ComparisonOperatorDefinition> {
let numeric_comparison_operators = BTreeMap::from_iter([
("_eq".into(), ComparisonOperatorDefinition::Equal),
Expand Down
8 changes: 6 additions & 2 deletions crates/calcite-schema/src/configuration.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Configuration for the connector.

use std::path::{Path, PathBuf};

use crate::environment::Environment;
use crate::error::{
MakeRuntimeConfigurationError, ParseConfigurationError,
Expand All @@ -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()
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/calcite-schema/src/jvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ static CONFIG: OnceCell<Mutex<ParsedConfiguration>> = OnceCell::new();
/// }
/// ```
// ANCHOR: get_jvm
#[tracing::instrument(skip())]
#[tracing::instrument(skip(), level=Level::INFO)]
pub fn get_jvm() -> &'static Mutex<JavaVM> {
{
let jvm = JVM.get().expect("JVM is not set up.");
Expand Down Expand Up @@ -69,7 +69,7 @@ pub fn get_jvm() -> &'static Mutex<JavaVM> {
/// 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
Expand Down
2 changes: 1 addition & 1 deletion crates/calcite-schema/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CollectionName, TableMetadata> {
let map = {
let jvm = get_jvm().lock().unwrap();
Expand Down
5 changes: 2 additions & 3 deletions crates/calcite-schema/src/scalars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -67,7 +66,7 @@ use crate::{aggregates, comparators};
/// }
/// ```
// ANCHOR: scalars
#[tracing::instrument(skip())]
#[tracing::instrument(skip(), level=Level::INFO)]
pub fn scalars() -> BTreeMap<ScalarTypeName, ScalarType> {
let string_comparison_operators =
comparators::string_comparators(&comparators::numeric_comparators("VARCHAR".into()));
Expand Down
2 changes: 1 addition & 1 deletion crates/calcite-schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<SchemaResponse, Box<dyn Error>> {
let data_models = get_models(&calcite_ref);
let scalar_types = scalars::scalars();
Expand Down
3 changes: 3 additions & 0 deletions crates/calcite-schema/src/version.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use tracing::{Level};

#[derive(Debug, Copy, Clone)]
pub enum VersionTag {
Version3,
Expand All @@ -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<String> {
match version {
VersionTag::Version3 => Some(
Expand Down
3 changes: 3 additions & 0 deletions crates/calcite-schema/src/version5.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions crates/connectors/ndc-calcite/src/calcite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub type Row = IndexMap<FieldName, RowFieldValue>;
/// 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();
Expand Down Expand Up @@ -115,7 +115,7 @@ fn parse_to_row(data: Vec<String>) -> Vec<Row> {
/// }
/// ```
// 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,
Expand Down
2 changes: 2 additions & 0 deletions crates/connectors/ndc-calcite/src/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use ndc_models::{
Capabilities, CapabilitiesResponse, LeafCapability, MutationCapabilities,
NestedFieldCapabilities, QueryCapabilities, RelationshipCapabilities,
};
use tracing::{Level};

/// Calculates the capabilities of the Calcite system.
///
Expand All @@ -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(),
Expand Down
24 changes: 12 additions & 12 deletions crates/connectors/ndc-calcite/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<models::RowSet, QueryError> {
Expand Down Expand Up @@ -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<Row>, sub_relationship: &Relationship) -> Result<Value, QueryError> {
let relationship_value: Value = rows_data.into_iter().map(|row| {
let mut row_values: Vec<Value> = Vec::new();
Expand All @@ -152,7 +152,7 @@ fn generate_value_from_rows(rows_data: &Vec<Row>, 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()
Expand All @@ -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<Option<Vec<Row>>, QueryError> {
if let Some(phrase) = &query_components.select {
if phrase.is_empty() && !query_components.final_aggregates.is_empty() {
Expand All @@ -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<Option<IndexMap<FieldName, Value>>, QueryError> {
if let Some(phrase) = &query_components.aggregates {
if phrase.is_empty() {
Expand Down Expand Up @@ -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<Expression, QueryError> {
let (_, name) = pks[0].clone();
Ok(Expression::BinaryComparisonOperator {
Expand All @@ -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<Query>, predicate: Expression, pks: &Vec<(FieldName, FieldName)>) -> Result<Box<Query>, QueryError> {
let mut revised_query = query.clone();
revised_query.predicate = Some(predicate);
Expand All @@ -267,7 +267,7 @@ fn revise_query(query: Box<Query>, 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<ArgumentName, RelationshipArgument>, sub_relationship: &Relationship, revised_query: &Query) -> Result<Vec<Row>, QueryError> {
let fk_rows = orchestrate_query(QueryParams {
config: params.config,
Expand All @@ -282,7 +282,7 @@ fn execute_query(params: QueryParams, arguments: &BTreeMap<ArgumentName, Relatio
Ok(fk_rows.rows.unwrap())
}

#[tracing::instrument(skip(rows, field_name, fk_rows, pks, fks))]
#[tracing::instrument(skip(rows, field_name, fk_rows, pks, fks), level=Level::DEBUG)]
fn process_object_relationship(rows: Vec<Row>, field_name: &FieldName, fk_rows: &Vec<Row>, pks: &Vec<(FieldName, FieldName)>, fks: &Vec<&FieldName>) -> Result<Option<Vec<Row>>, QueryError> {
let modified_rows: Vec<Row> = 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);
Expand Down Expand Up @@ -315,7 +315,7 @@ fn process_object_relationship(rows: Vec<Row>, 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<Vec<Row>>, field_name: &FieldName, fk_rows: &Vec<Row>, pks: &Vec<(FieldName, FieldName)>, fks: &Vec<&FieldName>, query: &Query) -> Result<Option<Vec<Row>>, QueryError> {
let modified_rows: Vec<Row> = 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);
Expand Down Expand Up @@ -349,7 +349,7 @@ fn process_array_relationship(rows: Option<Vec<Row>>, 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<String, Value>, value: &mut RowFieldValue) -> Result<(), QueryError> {
rowset.insert("aggregates".to_string(), Value::Null);
if !child_rows.is_empty() {
Expand Down
4 changes: 2 additions & 2 deletions crates/connectors/ndc-calcite/src/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<RelationshipName, Relationship>, arguments: &'a BTreeMap<ArgumentName, RelationshipArgument>, query: &'a Query, variables: &'a BTreeMap<VariableName, Value>) -> Result<QueryComponents, QueryError> {
let mut argument_values = BTreeMap::new();
for (argument_name, argument_value) in arguments {
Expand Down
3 changes: 2 additions & 1 deletion crates/values/src/is_running_in_container.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::env;
use std::path::Path;
use tracing::{Level};

/// Checks if the code is running inside a container.
///
Expand All @@ -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()
}

0 comments on commit 50674d6

Please sign in to comment.