Skip to content

Conversation

@lauzadis
Copy link
Contributor

@lauzadis lauzadis commented Feb 4, 2025

Issue #

Description of changes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

}

named("jvmTest") {
findByName("jvmTest")?.run {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: this change was necessary because the new logging-crt module does not have a JVM source set, causing errors like "jvmTest does not exist"

@github-actions

This comment has been minimized.

@lauzadis lauzadis added the no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly. label Feb 5, 2025
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link

Affected Artifacts

Changed in size
Artifact Pull Request (bytes) Latest Release (bytes) Delta (bytes) Delta (percentage)
http-test-jvm.jar 61,239 58,665 2,574 4.39%
crt-util-jvm.jar 21,451 20,952 499 2.38%
runtime-core-jvm.jar 844,845 827,499 17,346 2.10%
aws-signing-tests-jvm.jar 456,688 456,627 61 0.01%
test-suite-jvm.jar 96,935 97,211 -276 -0.28%

@lauzadis lauzadis marked this pull request as ready for review March 19, 2025 02:51
@lauzadis lauzadis requested a review from a team as a code owner March 19, 2025 02:51
public class CrtLogger(public val name: String, public val config: CrtConfig) :
WithCrt(),
Logger {
override fun trace(t: Throwable?, msg: MessageSupplier): Unit = log(CrtLogLevel.Trace, msg())
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Can we pass in the t to the crt log function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we can't. This is invoking a CRT function AWS_LOGF which has no concept of exceptions / throwables

}

// CRT logger does not support setting key-value pairs
override fun setKeyValuePair(key: String, value: Any) { }
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Is this not supported yet or are there no plans to support it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No plans to support it

@lauzadis lauzadis merged commit ad639f2 into kn-main Mar 21, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge-artifact-size-increase no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants