Skip to content

Add new LogBox configuration option to disallow serializing complex objectts#615

Closed
elpete wants to merge 1 commit into
developmentfrom
disallow_complex_objects_in_extraInfo
Closed

Add new LogBox configuration option to disallow serializing complex objectts#615
elpete wants to merge 1 commit into
developmentfrom
disallow_complex_objects_in_extraInfo

Conversation

@elpete
Copy link
Copy Markdown
Contributor

@elpete elpete commented Jul 17, 2025

Description

A new top level config item named allowSerializingComplexObjects controls whether LogBox will attempt to serialize and log out complex objects in extraInfo arguments. Defaults to true. Turning this to false can catch instances where you are using lots of processing time converting objects to XML or JSON unnecessarily. Removing these can increase performance and stability of running applications.

Jira Issues

https://ortussolutions.atlassian.net/browse/LOGBOX-83

Type of change

Please delete options that are not relevant.

  • Bug Fix
  • Improvement
  • New Feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • My code follows the style guidelines of this project cfformat
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

…bjects

A new top level config item named `allowSerializingComplexObjects` controls
whether LogBox will attempt to serialize and log out complex objects
in `extraInfo` arguments.  Defaults to `true`.  Turning this to `false`
can catch instances where large you using lots of processing time converting
objects to XML or JSON unnecessarily.  Removing these can increase
performance and stability of running applications.
@elpete elpete requested a review from lmajano July 17, 2025 16:57
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 17, 2025

Test Results

   42 files  +    6    936 suites  +133   7m 31s ⏱️ +56s
1 251 tests +    2  1 251 ✅ +    3    0 💤 ± 0  0 ❌ ±0 
7 326 runs  +1 187  6 894 ✅ +1 113  432 💤 +73  0 ❌ ±0 

Results for commit 3fb11d5. ± Comparison against base commit c3afabf.

This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.
tests.specs.logging.appenders.FileAppenderTest ‑ tests.specs.logging.appenders.FileAppenderTest
tests.specs.logging.LoggerTest ‑ Logger can control serializing complex objects allows serializing complex objects by default
tests.specs.logging.LoggerTest ‑ Logger can control serializing complex objects can disallow serializing complex objects
tests.specs.logging.LoggerTest ‑ Logger can control serializing complex objects can find complex objects inside structs

♻️ This comment has been updated with latest results.

@lmajano lmajano requested a review from Copilot July 18, 2025 12:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new configuration option allowSerializingComplexObjects to LogBox that controls whether complex objects can be serialized in the extraInfo parameter of log messages. When set to false, it can help identify performance bottlenecks caused by unnecessary object serialization during logging.

  • Added allowSerializingComplexObjects configuration property with default value of true
  • Implemented validation logic to detect and prevent complex object serialization when disabled
  • Added comprehensive test coverage for the new functionality including nested object detection

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
system/logging/config/LogBoxConfig.cfc Adds the new configuration property and parsing logic
system/logging/Logger.cfc Implements complex object detection and validation with exception throwing
system/logging/LogBox.cfc Passes the configuration setting to logger instances
tests/specs/logging/LoggerTest.cfc Adds test cases for the new serialization control functionality

Comment thread tests/specs/logging/LoggerTest.cfc
Comment thread system/logging/Logger.cfc
Comment on lines +498 to +504
return checkForComplexObjects( v );
} );
}

if ( isStruct( arguments.value ) ) {
return structSome( arguments.value, function( k, v ){
return checkForComplexObjects( v );
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

The recursive check for complex objects could cause performance issues with deeply nested or large data structures. Consider adding a depth limit or size limit to prevent excessive recursion that could impact logging performance.

Suggested change
return checkForComplexObjects( v );
} );
}
if ( isStruct( arguments.value ) ) {
return structSome( arguments.value, function( k, v ){
return checkForComplexObjects( v );
return checkForComplexObjects( v, depth + 1 );
} );
}
if ( isStruct( arguments.value ) ) {
return structSome( arguments.value, function( k, v ){
return checkForComplexObjects( v, depth + 1 );

Copilot uses AI. Check for mistakes.
Comment thread system/logging/Logger.cfc
return false;
}

if ( isArray( arguments.value ) ) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In all reality, if we are going to NOT allow anything complex, I would say don't even check for here down, why go through the trouble of iterate through each of the values, this is time consuming, error proned and just slow.

Comment thread system/logging/Logger.cfc
} );
}

if ( isStruct( arguments.value ) ) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above

Comment thread system/logging/Logger.cfc
return canLog( this.logLevels.DEBUG );
}

private boolean function checkForComplexObjects( required any value ){
Copy link
Copy Markdown
Member

@lmajano lmajano Jul 18, 2025

Choose a reason for hiding this comment

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

document the function, no excuses now, use chatgpt

Comment thread system/logging/Logger.cfc
}

// CFML Exceptions are safe
if ( isCFMLException( arguments.value ) ) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove the cfml, this is now. boxlang + cfml engine, use abstractions: isException()

Comment thread system/logging/Logger.cfc
return true;
}

private boolean function isCFMLException( required any value ){
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change method name to isException and document it

@lmajano
Copy link
Copy Markdown
Member

lmajano commented Sep 19, 2025

Merged Manually gracias

@lmajano lmajano closed this Sep 19, 2025
@lmajano lmajano deleted the disallow_complex_objects_in_extraInfo branch September 23, 2025 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants