HeartBeat Feature Implementation #631

dhaval24 · 2018-03-31T19:31:17Z

We would like to introduce HeartBeat feature.

What is Heartbeat?

Heartbeat in computer science is a periodic signal generated by software or hardware to indicate the normal operation or perform some synchronization. (https://en.wikipedia.org/wiki/Heartbeat_(computing))

HeartBeat in Application Insights

HeartBeat in Application Insights would send out some minimal amount of information on a scheduled interval to assist in high level monitoring of application health and would provide some meta data like OS type, Java version, Application Insights SDK version, Website Name and Host (If running on Azure WebApps) and is extensible to add more information should your application have heartbeat specific requirements.

HeartBeat Default Payload

Default Heartbeat payload will contain following information :
HeartBeat would be MetricTelemetry Item, a successful healthy heartbeat would have Metric Value = 0
It would contain following custom Dimensions -
OS Name - Indicates the name of operating system on which application is running
Java Version - Indicates the JDK version which is used by the application.
SDK Version - Indicates the version of ApplicationInsights Java SDK being used
processSessionId - a random guid which represents each running session of application

If Application is running on Azure WebApps it would also have following information -
Website Name - Name of the website
Host Stamp Name - Name of the host

Core Components

HeartBeatProviderInterface - Defines the implementation of concrete HeartBeat Provider. Should a user want to create a custom heartbeat provider they should implement the contract of this interface.

HearBeatProvider - Default Concrete Implementation of HeartBeatProviderInterface for transmitting HeartBeat Metric

HeartBeatDefaultPayloadProviderInterface - Interface which defines HeartBeatPayloadProvider and mechanism to add heartbeat property via separate callable. If a user would like to add custom properties which belong to specific group, implementing this interface is recommended.

BaseDefaultHeartbeatPropertyProvider - Concrete implementation of HeartBeatDefaultPayloadProviderInterface for default properties like SDK version, OS name, Java Version and processSessionId

WebAppsDefaultHeartbeatProvider - Concrete implementation of HeartBeatDefaultPayloadProviderInterface for default azure webapps properties.

HeartBeatModule - Concrete Implementation of TelemetryModule interface which composes HeartBeatProvider class to transmit heartbeat periodically. This module is registered by default even with bare minimum telemetry configuration.

HeartBeatPropertyPayload - A container for holding heartbeat property value and meta data about if it has updated and health.

Can you turn of HeartBeat ?

Yes, heartbeat can be turned off but it is highly not recommended to do so.

What is the default interval at which heatbeat is sent?

The, default interval at which heartbeat will be sent is 15 minutes.

What is the minimum interval which can be configured to send heartbeat?

The, minimum interval which can be configured is 5 seconds.

Can we override default heartbeat properties?

No, you cannot override default heartbeat properties. The are default and vital.

Can you exclude default heartbeat providers?

Yes, you can but it is not recommended

For significant contributions please make sure you have completed the following items:

Changes in public surface reviewed
CHANGELOG.md updated

…ng HeartbeatDefaultPayload

… sometest, added more tests, added mocks as required

…e meaningful, corrected spelling errors

…ged counter to atomic long for threadsafe

dhaval24 · 2018-04-03T00:57:44Z

A general comment on the heartbeat interval. Currently there is no maximum limit on heartbeat interval. I believe C# implementation also does not have the same, but I would suggest having an upper limit to make sure that we always receive heartbeat in some period.

Dmitry-Matveev

Small comments below, I'd recommend adding Derek (@d3r3kk) into the review as well.

Dmitry-Matveev · 2018-04-03T00:30:48Z

...in/java/com/microsoft/applicationinsights/internal/config/TelemetryConfigurationFactory.java

@@ -173,7 +176,13 @@ private void setTelemetryModules(ApplicationInsightsXmlConfiguration appConfigur
            ReflectionUtils.loadComponents(TelemetryModule.class, modules, configurationModules.getAdds());
        }

+        //if heartbeat module is not loaded, load heartbeat module
+        if (!isHeartBeatModuleAdded(modules)) {
+            addHeartBeatModule(configuration);


Is this behavior common across AI SDKs implementing heartbeat? I would not want customers remember in which SDK they would need to add HeartBeat explicitly if replacing modules vs. where it will be added by default even if they specifically didn't mention it.

@Dmitry-Matveev in .net SDK DiagnosticModule is used to enable HeartBeat module and I believe DiagnosticModule is turned on by default. However, in Java SDK there is no diagnostic module and if we want to enable heartbeat module by default if not specified in the configuration file I think this would be the way. If you have better option I would like to hear that.
As far as behavior will go, both the SDKs would be consistent that means unless user has turned the HeartBeat module off explicitly in configuration it will be turned on by default with default settings.

Dmitry-Matveev · 2018-04-03T01:09:23Z

...va/com/microsoft/applicationinsights/internal/heartbeat/WebAppsDefaultHeartbeatProvider.java

+        for (String fieldName : enabledProperties) {
+          try {
+            switch (fieldName) {
+              case "website_site_name":


Do you read it for every heartbeat? In .NET SDK, Telemetry Initializers cached the value, and, therefore, were missing slot swap when the name changes. It looks like you read it every time here, so heartbeat will pick new name up.

No, I am not reading it everytime, its cached in final vairable. But this is a good point, if this is a scenario then we might have to read always or if not atleast every X minutes.

See microsoft/ApplicationInsights-dotnet-server#872 for the issue tracking the slot-swap problem.

@d3r3kk and @Dmitry-Matveev I have updated the code to remove the caching of environment variable to querying every-time. I believe this won't be a performance overhead per say because environment variable because in case of windows I believe environment variables are stored in Registry and in Linux they are stored in /etc/environment which is also a very fast access location.

dhaval24 · 2018-04-03T01:27:10Z

Somehow Github doesn't find @d3r3kk in reviewers list. Please feel free to add your points here Derek.

d3r3kk · 2018-04-04T14:15:29Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+          setResult = true;
+        }
+        else {
+          throw new Exception("heartbeat property cannot be set without adding it first, or heartbeat"


Given the chance to do this a second time, I would allow Add or Set to actually add new fields. Perhaps issue a logger warning if a user is Adding new values via Set, if there is any real danger of doing this.

Originally, I was concerned that allowing adds to occur at any time might encourage the abuse of the Heartbeat for what is more appropriate to do with TrackEvent calls. And with the heartbeats properties being around for the entire duration of a program's runtime, I was slightly concerned with memory fragmentation should any abuse occur. However, I doubt there is much of a real problem there.

@d3r3kk Actually, I would like to keep the convention of add by default for adding and set by default for modifying. So keeping this in mind logically add should come before modifying (aka set in this case). I agree this is a bit opinionated and restricting the users but setting the convention I think will prove to be better in long run. Lot's of tunable knobs sometimes lead to spaghetti :)

@dhaval24, I'm inclined to agree with @d3r3kk. See the methods defined in Java's Map class. There could be places in the code where you won't know if a key has been added or not. I'm not a fan of the if containsKey set(x=y) else add(x=y) pattern when you can have a simple set(x=y).

Just as in Java's Map, putIfAbsent can be nice, if there's a precedent.

@littleaj this is addressed

d3r3kk · 2018-04-04T14:21:43Z

Added some comments, and enjoyed reading through this - I haven't looked at Java for a very long time. I don't see any obvious problems with this change, and the API is aligned nicely with the .NET SDK for the same feature which is probably for the best.

… failures

dhaval24 · 2018-04-04T21:13:18Z

@d3r3kk thank you very much for reviewing and comments. I have addressed one of it and left my opinion on another. I am glad you enjoyed reading through this.

dhaval24 · 2018-04-11T00:23:28Z

I made some changes to accommodate auto configuration of HeartBeat in SpringBoot starter

dhaval24 · 2018-04-13T21:08:03Z

@littleaj I have tried to address maximum of your comments in my latest commit. Take a look hopefully those which are not addressed are not very critical at the moment.

littleaj · 2018-04-13T22:31:41Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatModule.java

+      synchronized (lock) {
+        if (!isInitialized) {
+          this.heartBeatProviderInterface.initialize(configuration);
+          InternalLogger.INSTANCE.info("heartbeat is enabled");


I don't think this change will work. isHeartbeatEnabled() uses the heartbeatProviderInterface, but the provider doesn't get the config values until it's initialized. Right?

littleaj · 2018-04-13T22:36:39Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

@@ -133,10 +135,10 @@ public boolean addHeartBeatProperty(String propertyName, String propertyValue,
             payload.setPayloadValue(propertyValue);
             heartbeatProperties.put(propertyName, payload);
             isAdded = true;
-             InternalLogger.INSTANCE.trace("added heartbeat property");
+             InternalLogger.INSTANCE.trace("added heartbeat property %s - %s", propertyName, propertyValue);


Hyphen is an interesting choice. Usually you see %s:%s or %s=%s to indicate key=value.

This is nit: I think should be fine :)

littleaj · 2018-04-13T22:41:57Z

...a/com/microsoft/applicationinsights/internal/heartbeat/DefaultHeartbeatPropertyProvider.java

@@ -145,7 +152,7 @@ private String getSdkVersion() {
   * Gets the OS version on which application is running.
   * @return String representing OS version
   */
-  private String getOsType() {
+  private String getOsVersion() {


Do we want the OS version? If so, you should explore the difference between os.name and os.version (i.e. os.name does not contain version information).

I also found a few articles stating that os.name doesn't have the correct value when on Windows 10. Perhaps there is a different way to find this information.

We can amend it, if there is a better way though I am not aware of one. This is meta data which is not very very vital though.

littleaj · 2018-04-13T22:46:01Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

@@ -88,6 +90,8 @@ public HeartBeatProvider() {
    this.heartbeatProperties = new ConcurrentHashMap<>();
    this.isEnabled = true;
    this.heartbeatsSent = new AtomicLong(0);
+    this.propertyUpdateService = Executors.newCachedThreadPool(ThreadPoolUtils.createDaemonThreadFactory(HeartBeatProvider.class));


This makes these two thread pools indistinguishable. You may need another method in ThreadPool utils to specify a custom name. #Resolved

littleaj · 2018-04-13T22:52:54Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+  }
+
+  @Override
+  public boolean addHeartBeatProperty(String propertyName, String propertyValue,


I must say, I have strong feelings against this add/set property pattern. I don't understand why we need to throw if someone is overwriting a property with add or setting a new property with set. Please explain why we are doing it this way.

@littleaj I believe it adds more readability and easier to distinguish operations. I know that it is not common in Java's map class however at the same time I also do feel that being some what opinionated here is not causing any blockage. Unless there is a mistake in documentation I don't think user will play around with this.

One knob is simpler than two knobs; and there's no reason to be this strict. That's all I'm saying.

@littleaj fair enough. I was under impression to change the api to have only set() method instead of add() and set() both. But from the comment now I understand that the only ask here is to not throw the exception but log the message accordingly which is acceptable. I can make this changes too.

littleaj · 2018-04-13T22:54:11Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+             InternalLogger.INSTANCE.trace("added heartbeat property %s - %s", propertyName, propertyValue);
+        }
+        else {
+          throw new Exception("heartbeat property cannot be added twice. Please use setHeartBeatProperty instead to modify the value");


Please use a more specific Exception subclass. #Resolved

This is nice to have but I don't think this should block the PR. #Resolved

littleaj · 2018-04-13T22:57:26Z

@dhaval24, also address the failing unit test.

littleaj · 2018-04-13T23:03:33Z

...in/java/com/microsoft/applicationinsights/internal/config/TelemetryConfigurationFactory.java

@@ -119,7 +120,9 @@ public final void initialize(TelemetryConfiguration configuration) {
    private void setMinimumConfiguration(ApplicationInsightsXmlConfiguration userConfiguration, TelemetryConfiguration configuration) {


Will the heartbeat module always be added? It didn't look like it the way this method is called. #Resolved

Yes this module will always be added. #Resolved

@littleaj look into the method setTelemetryModules(). It adds HB module explicitly and also look atsetMininmumConfiguration(). This module would be present by default. #Resolved

@dhaval24 , Ok, I see.

I did notice that you can reduce some redundancy if you remove the call to addHeartbeatModule from setMinimumConfiguration. Since setTelemetryModules is always called and setMinimumConfiguration is only called when the XML config is missing, you don't need the add call in setMinimumConfiguration. #Resolved

@littleaj I need to add this for SpringBoot scenarios. Here it is not called because SpringBoot autoconfigures it using beans. #Resolved

Ahh, I see. I missed the return statements above.

In reply to: 181910995 [](ancestors = 181910995)

littleaj · 2018-04-13T23:04:35Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatModule.java

+ * @author Dhaval Doshi
+ * @since 03-30-2018
+ */
+public class HeartBeatModule implements TelemetryModule {


It appears this needs a no arg constructor as well.

We can have that ... but not needed. We can pass the null inside the parameterized constructor. There is a check there which handles it. I don't think we need to expose other constructor here if we can avoid.

littleaj · 2018-04-13T23:25:51Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+
+      //Submit task to set properties to dictionary using separate thread. we do not wait for the
+      //results to come out as some I/O bound properties may take time.
+      propertyUpdateService.submit(HeartbeatDefaultPayload.populateDefaultPayload(getExcludedHeartBeatProperties(),


Which properties are I/O bound? I wasn't able to find any. We may be able to remove the thread pool.

There are no properties which are IO bound today. But later on there will be when we will query ARM to get VM ID. I would rather keep it.

littleaj · 2018-04-13T23:32:37Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+      MetricTelemetry telemetry = (MetricTelemetry)gatherData();
+      telemetry.getContext().getOperation().setSyntheticSource(HEARTBEAT_SYNTHETIC_METRIC_NAME);
+      telemetryClient.trackMetric(telemetry);
+      heartbeatsSent.incrementAndGet();


This can be in the log line below. It returns the value after incrementing. Also, since it's in a lock it doesn't need to be atomic #Resolved

See other comment regarding the lock. If you remove the lock, you can keep the atomic int.

dhaval24 · 2018-04-14T00:22:27Z

@littleaj I have addressed your concerns on ThreadPool identification and fixed the broken tests.

…ng comments

dhaval24 · 2018-04-14T16:35:25Z

@littleaj all of your concerns must have been addressed with my recent changes. Please take a look and would be great if we can get this merged if no blockers are left.

littleaj · 2018-04-16T17:29:34Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+             InternalLogger.INSTANCE.trace("added heartbeat property %s - %s", propertyName, propertyValue);
+        }
+        else {
+          throw new IllegalStateException("heartbeat property cannot be added twice. Please use setHeartBeatProperty instead to modify the value");


Sorry I missed this in my first review. If you are throwing and catching in the same method based on a conditional, you can refactor it so that the statements in the catch block are moved to inside the else block (in place of throw). Another option would be to throw the exception out of this method. #Resolved

Statements and returns should just be fine. I will make this changes. #Resolved

littleaj · 2018-04-16T18:02:52Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+   * Creates and returns the heartbeat telemetry.
+   * @return Metric Telemetry which represent heartbeat.
+   */
+  private Telemetry gatherData() {


This should just return MetricTelemetry so you don't have to cast it. #Resolved

Fair, Okay this should be addressed. #Resolved

littleaj · 2018-04-16T18:05:03Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+    return new Runnable() {
+      @Override
+      public void run() {
+        if (isEnabled) {


I don't follow this conditional. Will the thread start even when it's disabled? #Resolved

Well actually it is unnecessary. But it doesn't hurt either, so I would not mind just keeping it there. #Resolved

littleaj · 2018-04-16T18:06:10Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+   */
+  private void send() {
+
+    synchronized (lock) {


What is the lock guarding? The ConcurrentMap keeps the CRUD to it thread-safe. I think you can just remove the lock and keep the atomic int

@littleaj my intention here is to send heartBeats synchronously. Telemetry client's track() method sends Telemetry asynchronously which is not desired here. I wanted to guard against an untoward situation like this. I have removed the Atomic version to primitive as you mentioned we are already locking and therefore avoid overhead.

littleaj · 2018-04-16T18:06:57Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+      MetricTelemetry telemetry = (MetricTelemetry)gatherData();
+      telemetry.getContext().getOperation().setSyntheticSource(HEARTBEAT_SYNTHETIC_METRIC_NAME);
+      telemetryClient.trackMetric(telemetry);
+      heartbeatsSent.incrementAndGet();


See other comment regarding the lock. If you remove the lock, you can keep the atomic int.

dhaval24 · 2018-04-16T18:46:44Z

@littleaj I have done another round of edits for your comments. I think everything is now addressed and this should be good to go. Kindly please approve after checking.

dhaval24 · 2018-04-17T02:53:53Z

@littleaj fixed couple of remaining unaddressed feedback also. Let me know if there are anything else.

dhaval24

@littleaj I have carefully gone through all your comments, marked them with fixed in most of the cases. The only one which is left is about ScheduledThreadPool. Let me know which route is preferred for this case. See my comments on it.

…nstructor, simplified methods

…own logic to ThreadPoolUtils

d3r3kk · 2018-04-19T20:43:19Z

Please note that there is a relevant change in the dotnet repo here today:
Known sdk versions (added heartbeat): Microsoft/ApplicationInsights-Home#213
Update heartbeat properties to include internal sdk version: microsoft/ApplicationInsights-dotnet#782

This change will affect the 2.7.0-stable release of .Net in July, so you might want to track an issue separate from this PR.

littleaj · 2018-04-19T22:56:03Z

gradle/wrapper/gradle-wrapper.properties

@@ -1,4 +1,3 @@
-#Fri Feb 09 18:53:31 PST 2018


@dhaval24 The gradle related files don't need to be updated. (gradle-wrapper.properties, gradle-wrapper.jar, gradlew)

littleaj

make the few changes we discussed and ship it!

littleaj · 2018-04-19T23:07:41Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatModule.java

+  /**
+   * Flag to seek if module is initialized
+   */
+  private static boolean isInitialized = false;


@dhaval24 Let's make this volatile just to be safe:
https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

littleaj · 2018-04-19T23:10:38Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/HeartBeatProvider.java

+    this.heartbeatProperties = new ConcurrentHashMap<>();
+    this.isEnabled = true;
+    this.heartbeatsSent = 0;
+    SDKShutdownActivity.INSTANCE.register(this);


Move this as the last line of the constructor so if the shutdown is triggered before the constructor ends it won't NPE.

littleaj · 2018-04-19T23:14:14Z

core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/MiscUtils.java

+   * @param inputList list of string containing reserved keywords
+   * @return true if input word is present in the list ignoring the case.
+   */
+  static boolean containsIgnoreCase(String keyword, Set<String> inputList) {


remove unused method

littleaj · 2018-04-19T23:15:22Z

core/src/main/java/com/microsoft/applicationinsights/internal/util/ThreadPoolUtils.java

+     * @param timeout Max time to wait
+     * @param timeUnit Timeunit for timeout
+     */
+    public static void stop(ExecutorService executorService, long timeout, TimeUnit timeUnit) {


Delete the older method since ThreadPoolExecutor is an ExecutorService

dhaval24 · 2018-04-20T05:23:30Z

All the issues are now addressed. Merging to master.

dhaval24 added 12 commits March 26, 2018 18:11

Initial heartbeat feature commit

bb3576b

adding required interfaces and main class to spin up Metric thread

8553cbd

some refactoring, implementing initialize for HeartBeatProvider, addi…

1a0fa4e

…ng HeartbeatDefaultPayload

some refactoring adding properties collection for webapps

edef438

fix unlocking

aca4383

removing unnecessary variable creation, initial test cases added

607b656

adding more tests

f147ff9

added support for heartbeat module to be added as default, refactored…

3430d26

… sometest, added more tests, added mocks as required

added more test cases, some refactoring in tests, added javadocs

efa1ff2

Merge branch 'master' into HeartBeat

6bd5570

updating test to remove try catch and adding throws

baba3b9

updated changelog, added somemore java docs, renamed method to be mor…

883fd72

…e meaningful, corrected spelling errors

dhaval24 added the Enhancement label Mar 31, 2018

dhaval24 added this to the 2.1.0 milestone Mar 31, 2018

dhaval24 self-assigned this Mar 31, 2018

dhaval24 requested review from littleaj, SergeyKanzhelev, Dmitry-Matveev and grlima March 31, 2018 19:31

dhaval24 added 2 commits March 31, 2018 20:24

fix threadpool shutdown timing, add missing shutdown for a pool, chan…

912bae6

…ged counter to atomic long for threadsafe

fixing proper locking

51f0258

Dmitry-Matveev reviewed Apr 3, 2018

View reviewed changes

d3r3kk reviewed Apr 4, 2018

View reviewed changes

removing caching of environment variable to prevent hotswap data read…

704a11f

… failures

Heartbeat updates to accomodate autoconfig in SpringBoot

fd168aa

done some refactoring and polishing, addressed review comments

6b8fe30

littleaj suggested changes Apr 13, 2018

View reviewed changes

littleaj reviewed Apr 13, 2018

View reviewed changes

fix a broken test and polish

c31ff35

some behavior change in setProperties, polish docs, addressed remaini…

d844011

…ng comments

dhaval24 mentioned this pull request Apr 15, 2018

Azure Application Insights SpringBoot Starter #646

Merged

3 tasks

littleaj suggested changes Apr 16, 2018

View reviewed changes

littleaj reviewed Apr 16, 2018

View reviewed changes

littleaj suggested changes Apr 16, 2018

View reviewed changes

addressing leftover

c8cff0e

done some refactoring, removed extra logging and polish

63a6217

Renaming, removing volatile

1e4ad3e

dhaval24 commented Apr 17, 2018

View reviewed changes

dhaval24 added 2 commits April 17, 2018 16:40

mockito for tests, removed redundant lock, add containsKey, public co…

d33177c

…nstructor, simplified methods

refactor to use factoryMethod for single thread executor, moved shutd…

757f0d1

…own logic to ThreadPoolUtils

littleaj reviewed Apr 19, 2018

View reviewed changes

littleaj approved these changes Apr 19, 2018

View reviewed changes

some minor changes and reverting gradle files changed

7531583

dhaval24 merged commit 7e37691 into master Apr 20, 2018

dhaval24 deleted the HeartBeat branch September 16, 2018 15:52

		@@ -119,7 +120,9 @@ public final void initialize(TelemetryConfiguration configuration) {
		private void setMinimumConfiguration(ApplicationInsightsXmlConfiguration userConfiguration, TelemetryConfiguration configuration) {

HeartBeat Feature Implementation #631

HeartBeat Feature Implementation #631

Conversation

dhaval24 commented Mar 31, 2018 • edited Loading

What is Heartbeat?

HeartBeat in Application Insights

HeartBeat Default Payload

Core Components

Can you turn of HeartBeat ?

What is the default interval at which heatbeat is sent?

What is the minimum interval which can be configured to send heartbeat?

Can we override default heartbeat properties?

Can you exclude default heartbeat providers?

dhaval24 commented Apr 3, 2018 • edited Loading

Dmitry-Matveev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhaval24 Apr 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhaval24 commented Apr 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d3r3kk commented Apr 4, 2018

dhaval24 commented Apr 4, 2018

dhaval24 commented Apr 11, 2018

dhaval24 commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littleaj Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littleaj Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

dhaval24 Apr 13, 2018 • edited by littleaj Loading

Choose a reason for hiding this comment

littleaj commented Apr 13, 2018

littleaj Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

dhaval24 Apr 14, 2018 • edited by littleaj Loading

Choose a reason for hiding this comment

dhaval24 Apr 14, 2018 • edited by littleaj Loading

Choose a reason for hiding this comment

littleaj Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

dhaval24 Apr 16, 2018 • edited by littleaj Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littleaj Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhaval24 commented Apr 14, 2018

dhaval24 commented Apr 14, 2018

littleaj Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

dhaval24 Apr 16, 2018 • edited by littleaj Loading

Choose a reason for hiding this comment

littleaj Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

dhaval24 Apr 16, 2018 • edited by littleaj Loading

Choose a reason for hiding this comment

littleaj Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

dhaval24 Apr 16, 2018 • edited by littleaj Loading

dhaval24 commented Mar 31, 2018 •

edited

Loading

dhaval24 commented Apr 3, 2018 •

edited

Loading

dhaval24 Apr 3, 2018 •

edited

Loading

littleaj Apr 13, 2018 •

edited

Loading

littleaj Apr 13, 2018 •

edited

Loading

dhaval24 Apr 13, 2018 •

edited by littleaj

Loading

littleaj Apr 13, 2018 •

edited

Loading

dhaval24 Apr 14, 2018 •

edited by littleaj

Loading

dhaval24 Apr 14, 2018 •

edited by littleaj

Loading

littleaj Apr 16, 2018 •

edited

Loading

dhaval24 Apr 16, 2018 •

edited by littleaj

Loading

littleaj Apr 13, 2018 •

edited

Loading

littleaj Apr 16, 2018 •

edited

Loading

dhaval24 Apr 16, 2018 •

edited by littleaj

Loading

littleaj Apr 16, 2018 •

edited

Loading

dhaval24 Apr 16, 2018 •

edited by littleaj

Loading

littleaj Apr 16, 2018 •

edited

Loading

dhaval24 Apr 16, 2018 •

edited by littleaj

Loading