Skip to content

Conversation

@sivatarunp
Copy link
Owner

@sivatarunp sivatarunp commented Jun 29, 2021

  1. Removed dependancy on test containers
  2. Removed the minimizeJar option in dependencies (maven shaded plugin). This was creating missing dependencies while running the jar
  3. Added dependency and configuration for log4j2
  4. Added provision for extra spark configuration which can help us in scaling
  5. Removed orphan Span counter calculations
  6. Replaces the tags with @ to . because of which few metrics are being improper
  7. Making Dynamo Table name configurable and removing windowing

@sivatarunp sivatarunp changed the title Kinesis source Adding changes for prod deployment Jun 29, 2021
Copy link

@RashmiRam RashmiRam left a comment

Choose a reason for hiding this comment

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

Let's discuss this on how to make this main more readable.

String region = Regions.getCurrentRegion()!=null ? Regions.getCurrentRegion().getName()
: getPropOrEnv("AWS_REGION", Regions.US_EAST_1.getName());

String service_endpoint = getPropOrEnv("KINESIS_ENDPOINT", "https://kinesis.us-east-1.amazonaws.com");

Choose a reason for hiding this comment

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

throw error if not provided. We can't try to read from other region and all right.

* @throws IOException
*/
public static void main(String[] args) throws InterruptedException, IOException {
PropertyConfigurator.configure("/data/log4j.properties"); // can make it configurable

Choose a reason for hiding this comment

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

Why do we have to do like this? Log4j2 will look for config file specified in a pre defined env var by default. We can set that env var to our log4j2.xml or log4j2.yml file when we run this jar.

<dependency>
    <groupId>org.apache.logging.log4j</groupId>
    <artifactId>log4j-api</artifactId>
    <version>2.14.1</version>
</dependency>
<dependency>
    <groupId>org.apache.logging.log4j</groupId>
    <artifactId>log4j-core</artifactId>
    <version>2.14.1</version>
</dependency>

https://logging.apache.org/log4j/2.x/manual/configuration.html

.setAppName("Trace DSL")
.setMaster(getPropOrEnv("SPARK_MASTER", "local[*]"))
.set("spark.testing.memory", getPropOrEnv("SPARK_MEMORY", "471859200"));
.set("spark.testing.memory", getPropOrEnv("SPARK_MEMORY", "4073741824"))

Choose a reason for hiding this comment

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

Move all defaults to constant and use it.

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.

2 participants