feat: Sending configuration to mixpanel #1504

adamfilipow92 · 2021-01-14T22:06:50Z

Fixes #1503

Test Plan

How do we know the code works?

Flank sending configuration to mixpanel
If integration tests or unit sending configuration disabled
Flag to disable sending statistics works
environment-variables, paths to files anonymized (maybe something more?)

Checklist

Sending usage statistics to mixpanel
Flag to disable usage statistic
Disable usage statistic in tests
Anonymized sensitive data
Unit tested
Integration tests updated
respects the same analytics opt out as gcloud CLI.

github-actions · 2021-01-14T22:07:04Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

github-actions · 2021-01-14T22:14:36Z

Timestamp: 2021-01-20 16:44:37
Buildscan url for ubuntu-workflow run 498716014
https://gradle.com/s/mibklz5ddczoc

pawelpasterz · 2021-01-15T06:30:15Z

I think we should not send statistics from developer environment (our, local, manual tests could spoil statistics). DIsabling for IT or UT is great but I think it won't be enough. Statistics should be only from official releases?
This is an open question.

pawelpasterz · 2021-01-15T06:47:29Z

I think we should not send statistics from developer environment (our, local, manual tests could spoil statistics). DIsabling for IT or UT is great but I think it won't be enough. Statistics should be only from official releases?
This is an open question.

On the second thought, it should be possible to filter local_snapshot statistics so it should not be an issue (at least I think)

piotradamczyk5 · 2021-01-15T11:07:52Z

I think we should not send statistics from developer environment (our, local, manual tests could spoil statistics). DIsabling for IT or UT is great but I think it won't be enough. Statistics should be only from official releases?
This is an open question.

On the second thought, it should be possible to filter local_snapshot statistics so it should not be an issue (at least I think)

we could make a flag and store it in FlankProperties 😃

bootstraponline · 2021-01-16T15:47:49Z

I think the uuid opt out should also apply to mixpanel

Flank respects the same analytics opt out as gcloud CLI.

echo "DISABLED" > ~/.gsutil/analytics-uuid

https://github.com/Flank/flank/blob/master/docs/error_monitoring.md#disable-bugsnag

test_runner/src/main/kotlin/ftl/analytics/SendUsageStatistics.kt

piotradamczyk5 · 2021-01-19T16:22:50Z

test_runner/src/main/kotlin/ftl/analytics/StatisticDataFilters.kt

+
+private val classesForStatistics = listOf(IArgs::class, AndroidArgs::class, IosArgs::class)
+
+private fun KClass<*>.ignoredMembersForStatistics() = findMembersWithAnnotation(IgnoreInStatistics::class)


please move below keysToRemove

Changed thanks! 👍

piotradamczyk5 · 2021-01-19T16:23:03Z

test_runner/src/main/kotlin/ftl/analytics/StatisticDataFilters.kt

+private val classesForStatistics = listOf(IArgs::class, AndroidArgs::class, IosArgs::class)
+
+private fun KClass<*>.ignoredMembersForStatistics() = findMembersWithAnnotation(IgnoreInStatistics::class)
+private fun KClass<*>.anonymousMembersForStatistics() = findMembersWithAnnotation(AnonymizeInStatistics::class)


please move below keysToAnonymize

Changed thanks! 👍

test_runner/src/main/kotlin/ftl/analytics/UsageStatisticsClient.kt

piotradamczyk5 · 2021-01-19T17:16:27Z

Please remeber to update flank.yml, flank.ios.yml, docs/index.md etc with new option

jan-goral

The implementation looks well! 👍 I have added a few suggestions but all related to one thing.

jan-goral · 2021-01-20T11:58:44Z

test_runner/src/main/kotlin/ftl/analytics/StatisticDataFilters.kt

+annotation class AnonymizeInStatistics
+
+internal val keysToRemove by lazy {
+    classesForStatistics.map { it.ignoredMembersForStatistics() }.flatten()


Suggested change

classesForStatistics.map { it.ignoredMembersForStatistics() }.flatten()

classesForStatistics.map(findMembersWithAnnotation(IgnoreInStatistics::class)).flatten()

jan-goral · 2021-01-20T11:59:18Z

test_runner/src/main/kotlin/ftl/analytics/StatisticDataFilters.kt

+    classesForStatistics.map { it.ignoredMembersForStatistics() }.flatten()
+}
+
+private fun KClass<*>.ignoredMembersForStatistics() = findMembersWithAnnotation(IgnoreInStatistics::class)


Suggested change

private fun KClass<*>.ignoredMembersForStatistics() = findMembersWithAnnotation(IgnoreInStatistics::class)

test_runner/src/main/kotlin/ftl/analytics/StatisticDataFilters.kt

…lementation

…t.kt Co-authored-by: piotradamczyk5 <65554637+piotradamczyk5@users.noreply.github.com>

Co-authored-by: piotradamczyk5 <65554637+piotradamczyk5@users.noreply.github.com>

adamfilipow92 assigned adamfilipow92 and piotradamczyk5 Jan 14, 2021

github-actions bot added the Feature label Jan 14, 2021

piotradamczyk5 reviewed Jan 19, 2021

View reviewed changes

test_runner/src/main/kotlin/ftl/analytics/SendUsageStatistics.kt Outdated Show resolved Hide resolved

piotradamczyk5 reviewed Jan 19, 2021

View reviewed changes

test_runner/src/main/kotlin/ftl/analytics/UsageStatisticsClient.kt Outdated Show resolved Hide resolved

adamfilipow92 marked this pull request as ready for review January 20, 2021 10:02

jan-goral reviewed Jan 20, 2021

View reviewed changes

Piotr Adamczyk and others added 14 commits January 20, 2021 14:49

style: Add Kotlin Lint to tests

475bcbf

Added support for detecting changes in configs, added mixpanel

de08709

Update Mixpanel.kt

9b76a59

Update Mixpanel.kt

dd80e71

Update Mixpanel.kt

f1d6474

Update Mixpanel.kt

c850a65

Fix crash on ios, set api token

62a74ed

Update Mixpanel.kt

6037185

add disable usage statistics flag

eec73bf

Usage statistics disabled in tests

b6f5a37

Update UsageStatistics.kt

feff626

Get ignored anonymized members by reflection, some changes in the imp…

32c6fd5

…lementation

Implementation changes, added and updated tests

b6a1ff0

Updated integration tests

c556a4c

adamfilipow92 and others added 6 commits January 20, 2021 13:50

respects the same analytics opt out as gcloud CLI

5f8bbaa

Update test_runner/src/main/kotlin/ftl/analytics/UsageStatisticsClien…

1d47664

…t.kt Co-authored-by: piotradamczyk5 <65554637+piotradamczyk5@users.noreply.github.com>

Update test_runner/src/main/kotlin/ftl/analytics/SendUsageStatistics.kt

8b4ef67

Co-authored-by: piotradamczyk5 <65554637+piotradamczyk5@users.noreply.github.com>

Implementation improvements

9960a37

Update docs

67e3c23

Update SendUsageStatistics.kt

2e5f40b

bootstraponline force-pushed the 1460-mix-panel-poc branch from 179e0aa to 2e5f40b Compare January 20, 2021 13:50

piotradamczyk5 force-pushed the master branch from 475bcbf to 5005b49 Compare January 20, 2021 13:53

piotradamczyk5 requested review from jan-goral and piotradamczyk5 January 20, 2021 14:02

fixed MR comments

c91227f

piotradamczyk5 force-pushed the 1460-mix-panel-poc branch from fd8fed7 to c91227f Compare January 20, 2021 14:03

piotradamczyk5 approved these changes Jan 20, 2021

View reviewed changes

jan-goral approved these changes Jan 20, 2021

View reviewed changes

mergify bot merged commit 6df778f into master Jan 20, 2021

mergify bot deleted the 1460-mix-panel-poc branch January 20, 2021 16:44

github-actions bot locked and limited conversation to collaborators Jan 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Sending configuration to mixpanel #1504

feat: Sending configuration to mixpanel #1504

adamfilipow92 commented Jan 14, 2021 •

edited

Loading

github-actions bot commented Jan 14, 2021 •

edited

Loading

github-actions bot commented Jan 14, 2021 •

edited

Loading

pawelpasterz commented Jan 15, 2021 •

edited

Loading

pawelpasterz commented Jan 15, 2021 •

edited

Loading

piotradamczyk5 commented Jan 15, 2021

bootstraponline commented Jan 16, 2021

piotradamczyk5 Jan 19, 2021

adamfilipow92 Jan 20, 2021

piotradamczyk5 Jan 19, 2021

adamfilipow92 Jan 20, 2021

piotradamczyk5 commented Jan 19, 2021

jan-goral left a comment

jan-goral Jan 20, 2021

jan-goral Jan 20, 2021


		private val classesForStatistics = listOf(IArgs::class, AndroidArgs::class, IosArgs::class)

		private fun KClass<*>.ignoredMembersForStatistics() = findMembersWithAnnotation(IgnoreInStatistics::class)

	classesForStatistics.map { it.ignoredMembersForStatistics() }.flatten()
	classesForStatistics.map(findMembersWithAnnotation(IgnoreInStatistics::class)).flatten()

feat: Sending configuration to mixpanel #1504

feat: Sending configuration to mixpanel #1504

Conversation

adamfilipow92 commented Jan 14, 2021 • edited Loading

Test Plan

Checklist

github-actions bot commented Jan 14, 2021 • edited Loading

github-actions bot commented Jan 14, 2021 • edited Loading

pawelpasterz commented Jan 15, 2021 • edited Loading

pawelpasterz commented Jan 15, 2021 • edited Loading

piotradamczyk5 commented Jan 15, 2021

bootstraponline commented Jan 16, 2021

piotradamczyk5 Jan 19, 2021

Choose a reason for hiding this comment

adamfilipow92 Jan 20, 2021

Choose a reason for hiding this comment

piotradamczyk5 Jan 19, 2021

Choose a reason for hiding this comment

adamfilipow92 Jan 20, 2021

Choose a reason for hiding this comment

piotradamczyk5 commented Jan 19, 2021

jan-goral left a comment

Choose a reason for hiding this comment

jan-goral Jan 20, 2021

Choose a reason for hiding this comment

jan-goral Jan 20, 2021

Choose a reason for hiding this comment

adamfilipow92 commented Jan 14, 2021 •

edited

Loading

github-actions bot commented Jan 14, 2021 •

edited

Loading

github-actions bot commented Jan 14, 2021 •

edited

Loading

pawelpasterz commented Jan 15, 2021 •

edited

Loading

pawelpasterz commented Jan 15, 2021 •

edited

Loading