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

feat: add config parameter to disable custom stringification #288

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

zermelo-wisen
Copy link
Contributor

@zermelo-wisen zermelo-wisen commented Aug 15, 2024

Fixes #287.

Introduced a new property appmap.event.disableValue to disable the custom stringification of objects during AppMap recordings. This change includes:

  • A DisableValue property in "Properties" class to toggle stringification.
  • Updates to the Value class to respect this property.
  • A new test case in DisabledValue to validate functionality when DisableValue is enabled.
  • An example class Example for testing and a bats script to verify the changes.

Summary of Changes

  1. Introduced a Configuration Property to Disable Stringification:
    • Added a new property DisableValue in the Properties class which can be toggled to disable the stringification of objects.
    • Default value for this property is set to false.
public class Properties {
   public static final Boolean DisableValue = resolveProperty("appmap.event.disableValue", false);
   public static final Integer MaxValueSize = resolveProperty("appmap.event.valueSize", 1024);
   
   // Other properties...
}
  1. Modified the Value Class to Utilize the New Property:
    • Updated the freeze method in the Value class to check the DisableValue property.
    • If DisableValue is true, the object’s value is set to a placeholder ("< disabled >").
public class Value {
   public Value freeze() {
     if (this.value != null) {
       if (Properties.DisableValue) {
         this.value = "< disabled >";
       } else {
         try {
           this.value = this.value.toString();
           if (Properties.MaxValueSize > 0 && this.value != null) {
             this.value = StringUtils.abbreviate((String) this.value, "...", Properties.MaxValueSize);
           }
         } catch (Throwable e) {
           Logger.println("failed to resolve value of " + this.classType);
           Logger.println(e.getMessage());
           this.value = "< invalid >";
         }
       }
     }
     return this;
   }
}
  1. Created a Test Case to Validate the New Property:
    • Added a new test class DisabledValue that generates an AppMap recording to verify the behavior when DisableValue is enabled.
import java.io.IOException;
import java.io.OutputStreamWriter;
import com.appland.appmap.record.Recorder;
import com.appland.appmap.record.Recording;

public class DisabledValue {
  public static void main(String[] argv) {
    final Recording recording = Recorder.getInstance().record(() -> {
      new Example().doSomething(new Example());
    });

    try {
      recording.readFully(true, new OutputStreamWriter(System.out));
    } catch (IOException e) {
      e.printStackTrace();
    }
  }
}
  1. Added an Example Class to Use in the Test:
    • Created the Example class with a toString method and a method doSomething that returns an instance of the Example class.
public class Example {
  @Override
  public String toString() {
    return "an Example";
  }

  public Example doSomething(Example other) {
    return other;
  }
}
  1. Updated Testing Setup and bats Test:
    • Configured a new bats test script to compile the test files and run the new test, verifying the DisableValue property functionality using jq to inspect the AppMap JSON output.
#!/usr/bin/env bats

load '../../build/bats/bats-support/load'
load '../../build/bats/bats-assert/load'
load '../helper'

sep="$JAVA_PATH_SEPARATOR"
AGENT_JAR="$(find_agent_jar)"
wd=$(getcwd)
test_cp="${wd}/test/event${sep}${wd}/build/classes/java/test"
java_cmd="java -javaagent:'${AGENT_JAR}' -cp '${test_cp}'"

setup() {
  javac -cp "${AGENT_JAR}${sep}${test_cp}" test/event/*.java
  cd test/event
  _configure_logging
}

@test "disabled value" {
  local cmd="${java_cmd} -Dappmap.event.disableValue=true DisabledValue"
  [[ $BATS_VERBOSE_RUN == 1 ]] && echo "cmd: $cmd" >&3

  local output
  output=$(eval "$cmd")

  echo "Output: $output" >&3

  echo "$output" | jq -e '.events[0] | select(.event=="call" 
    and .parameters[0].value == "< disabled >" 
    and .receiver.value == "< disabled >")'
  echo "$output" | jq -e '.events[1] | select(.event=="return" 
    and .return_value.value == "< disabled >")'
}

@apotterri apotterri self-requested a review August 15, 2024 20:31
@zermelo-wisen zermelo-wisen force-pushed the feat/disable-value-tostring-config branch from 652bea3 to 1eb2ecf Compare August 15, 2024 20:39
@@ -0,0 +1,11 @@
public class Example {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better for this to be in the com.appland.appmap.test.fixture package.

@zermelo-wisen zermelo-wisen force-pushed the feat/disable-value-tostring-config branch 2 times, most recently from 1bc1e26 to ac5ef0e Compare August 25, 2024 10:49
Introduced a new property "appmap.event.disableValue" to
disable the custom stringification of objects during AppMap
recordings. This change includes:

- A "DisableValue" property in "Properties" class to toggle
  stringification.
- Updates to the "Value" class to respect this property.
- A new test case in "DisabledValue" to validate functionality
  when "DisableValue" is enabled.
- An example class "Example" for testing and a "bats" script to
  verify the changes.
@apotterri apotterri force-pushed the feat/disable-value-tostring-config branch from ac5ef0e to cb18931 Compare August 27, 2024 11:35
Set GH_TOKEN so builds won't get rate limited.
@apotterri apotterri force-pushed the feat/disable-value-tostring-config branch from 45a4542 to 2b7ff80 Compare August 27, 2024 13:23
@apotterri apotterri merged commit 0bc2e91 into master Aug 27, 2024
6 checks passed
@apotterri apotterri deleted the feat/disable-value-tostring-config branch August 27, 2024 20:27
@appland-release
Copy link

🎉 This issue has been resolved in version 1.27.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding a flag to disable custom stringification
3 participants