Default environment #3
Conversation
|
|
||
| /** | ||
| * The LogGroup name to use. This will be ignored when using the | ||
| * Lambda scope. |
There was a problem hiding this comment.
I think you probably copied this from the other repos, but we can be explicit in the javadocs and say that this is really just for the CWAgent.
| @@ -0,0 +1,7 @@ | |||
| package software.amazon.awssdk.services.cloudwatchlogs.emf.config; | |||
|
|
|||
| class SystemWrapper { | |||
There was a problem hiding this comment.
The Configuration is a singleton and is created by EnvironmentConfigurationProvider. The Provider reads environment variables from System. It's difficult to mock the System to test the EnvironmentConfigurationProvider, so I used SystemWrapper to make it possible to mock System.
| @@ -65,4 +67,9 @@ void putMetric(String key, double value) { | |||
| } | |||
| return dimensions; | |||
| } | |||
|
|
|||
| String serialize() throws JsonProcessingException { | |||
| ObjectMapper mapper = new ObjectMapper(); | |||
There was a problem hiding this comment.
Do we need to create an ObjectMapper every time? Also, recommend looking at this issue: awslabs/aws-embedded-metrics-node#50
There was a problem hiding this comment.
Fixed. Added a property filter to filter out the _aws if no metrics have been defined.
| /** | ||
| * Establish a connection to a socket | ||
| */ | ||
| void connect(); |
There was a problem hiding this comment.
Do we want this on the interface? The socket client needs to manage some state anyways and connect can probably be encapsulated rather than require the sink to call it before sendMessage.
|
|
||
| @Override | ||
| public void sendMessage(String message) { | ||
| if (socket.isClosed() || shouldConnect) { |
There was a problem hiding this comment.
This is not a thread safe operation. Is this class used across threads?
There was a problem hiding this comment.
Yes, it's possible that this class is used across threads.
There was a problem hiding this comment.
In the Python lib, we address this using 2 instance-level locks for writes and connecting. https://github.com/awslabs/aws-embedded-metrics-python/blob/master/aws_embedded_metrics/sinks/tcp_client.py#L35
- Filter out '_aws' metadata object if no metrics have been added - Add lock for sendMessage method
Issue #, if available:
Description of changes:
TODO:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.