-
Notifications
You must be signed in to change notification settings - Fork 0
Allow clients to configure logging #69
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
base: develop
Are you sure you want to change the base?
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
7ea9701
to
0e6d823
Compare
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.
Pull Request Overview
This PR introduces configurable logging for the Stream Feeds Android client, allowing clients to provide custom loggers and configure HTTP logging levels.
- Adds
LoggingConfig
class to configure custom logger and HTTP logging level - Creates logger abstraction with
Logger
interface and level enum - Implements custom logger provider that bridges to core Android logging
- Updates sample app to demonstrate custom logging configuration
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
FeedsConfig.kt | Adds loggingConfig parameter with default configuration |
LoggingConfig.kt | New configuration class for logging settings |
Logger.kt | New logger interface with level enum for custom implementations |
HttpLoggingLevel.kt | New enum mapping HTTP logging levels to OkHttp interceptor levels |
CustomLoggerProvider.kt | Implementation bridging custom logger to core logging system |
LoggingFactory.kt | Factory functions for creating logger providers and HTTP interceptors |
Create.kt | Updates client creation to use configurable logging instead of hardcoded settings |
CustomLoggerProviderTest.kt | Test coverage for custom logger provider functionality |
LoginManager.kt | Sample app demonstrates custom logging configuration |
Comments suppressed due to low confidence (1)
stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/client/Create.kt:1
- The import
okhttp3.logging.HttpLoggingInterceptor
is removed but the functionality is now handled by the factory function. This is good cleanup as the direct usage has been abstracted away.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...s-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/client/Create.kt
Show resolved
Hide resolved
...oid-client/src/main/kotlin/io/getstream/feeds/android/client/api/logging/HttpLoggingLevel.kt
Outdated
Show resolved
Hide resolved
...oid-client/src/main/kotlin/io/getstream/feeds/android/client/api/logging/HttpLoggingLevel.kt
Outdated
Show resolved
Hide resolved
...t/src/main/kotlin/io/getstream/feeds/android/client/internal/logging/CustomLoggerProvider.kt
Outdated
Show resolved
Hide resolved
package io.getstream.feeds.android.client.api.logging | ||
|
||
/** A logger interface for logging messages at different levels. */ | ||
public interface Logger { |
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.
I assume it is intentional that we don't expose the logger
from core
, but we use a Feeds
-specific implementation?
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.
Yes! I went for this because the core logger is marked as internal api, but if we prefer to expose StreamLoggerProvider
& StreamLogger
instead we definitely can. No strong opinion on my side.
If we do that, we should also evaluate the naming, because it could be a bit confusing from a client perspective that we have those, but also similarly-named code (e.g. StreamLogger
) in the stream-log library.
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.
I see, thanks for the clarification! Let's keep it like this, as it would be easier to expose the core
logger later if needed, rather than having to hide it afterwards.
0e6d823
to
1091432
Compare
Goal
Allow clients to pass a custom logger & decide the http logging level.
Implementation
Introduce
LoggingConfig
that clients can use to pass a custom logger & the logging level. By default, we use core's Android logging & basic http logging.Testing
Modify the sample app to pass a custom logger and verify it's called.
Checklist