-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add LDConfig.sendEvents
option to disable all events
#414
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,14 +15,14 @@ protocol ClientServiceCreating { | |
service: DarklyServiceProvider, | ||
onSyncComplete: FlagSyncCompleteClosure?) -> LDFlagSynchronizing | ||
func makeFlagChangeNotifier() -> FlagChangeNotifying | ||
func makeEventReporter(service: DarklyServiceProvider) -> EventReporting | ||
func makeEventReporter(service: DarklyServiceProvider, onSyncComplete: EventSyncCompleteClosure?) -> EventReporting | ||
func makeEventReporter(config: LDConfig, service: DarklyServiceProvider) -> EventReporting | ||
func makeEventReporter(config: LDConfig, service: DarklyServiceProvider, onSyncComplete: EventSyncCompleteClosure?) -> EventReporting | ||
func makeStreamingProvider(url: URL, httpHeaders: [String: String], connectMethod: String, connectBody: Data?, handler: EventHandler, delegate: RequestHeaderTransform?, errorHandler: ConnectionErrorHandler?) -> DarklyStreamingProvider | ||
func makeEnvironmentReporter(config: LDConfig) -> EnvironmentReporting | ||
func makeThrottler(environmentReporter: EnvironmentReporting) -> Throttling | ||
func makeConnectionInformation() -> ConnectionInformation | ||
func makeDiagnosticCache(sdkKey: String) -> DiagnosticCaching | ||
func makeDiagnosticReporter(service: DarklyServiceProvider, environmentReporter: EnvironmentReporting) -> DiagnosticReporting | ||
func makeDiagnosticReporter(config: LDConfig, service: DarklyServiceProvider, environmentReporter: EnvironmentReporting) -> DiagnosticReporting | ||
func makeFlagStore() -> FlagMaintaining | ||
} | ||
|
||
|
@@ -66,12 +66,16 @@ final class ClientServiceFactory: ClientServiceCreating { | |
FlagChangeNotifier(logger: logger) | ||
} | ||
|
||
func makeEventReporter(service: DarklyServiceProvider) -> EventReporting { | ||
makeEventReporter(service: service, onSyncComplete: nil) | ||
func makeEventReporter(config: LDConfig, service: DarklyServiceProvider) -> EventReporting { | ||
makeEventReporter(config: config, service: service, onSyncComplete: nil) | ||
} | ||
|
||
func makeEventReporter(service: DarklyServiceProvider, onSyncComplete: EventSyncCompleteClosure? = nil) -> EventReporting { | ||
EventReporter(service: service, onSyncComplete: onSyncComplete) | ||
func makeEventReporter(config: LDConfig, service: DarklyServiceProvider, onSyncComplete: EventSyncCompleteClosure? = nil) -> EventReporting { | ||
if config.sendEvents { | ||
EventReporter(service: service, onSyncComplete: onSyncComplete) | ||
} else { | ||
NullEventReporter() | ||
} | ||
} | ||
|
||
func makeStreamingProvider(url: URL, | ||
|
@@ -121,8 +125,12 @@ final class ClientServiceFactory: ClientServiceCreating { | |
DiagnosticCache(sdkKey: sdkKey) | ||
} | ||
|
||
func makeDiagnosticReporter(service: DarklyServiceProvider, environmentReporter: EnvironmentReporting) -> DiagnosticReporting { | ||
DiagnosticReporter(service: service, environmentReporting: environmentReporter) | ||
func makeDiagnosticReporter(config: LDConfig, service: DarklyServiceProvider, environmentReporter: EnvironmentReporting) -> DiagnosticReporting { | ||
if config.sendEvents && !config.diagnosticOptOut { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we also add this bit to make it a little more efficient by not making a reporter at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The suspense between the two comments. I read the first one and I was like, "yes, but that indirectness scares me". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😆 yes, I hate that there are even two paths like this. But there are protocols that are depending on others in a cycle and so really teasing that out as much as I would prefer is probably more effort than it is worth. Especially since we're pushing for a larger re-work. |
||
DiagnosticReporter(service: service, environmentReporting: environmentReporter) | ||
} else { | ||
NullDiagnosticReporter() | ||
} | ||
} | ||
|
||
func makeFlagStore() -> FlagMaintaining { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change alone is sufficient to disable diagnostic events. If the cache doesn't exist, we won't collect diagnostic events.