fix: Logger settings for feature servers and updated logger for permi…

…ssion flow (feast-dev#4531) * Logger setting for feature servers Signed-off-by: Abdul Hameed <ahameed@redhat.com> * added/updated logger for permission flow Signed-off-by: Abdul Hameed <ahameed@redhat.com> * set defualt logger level for feature servers as WARNING Signed-off-by: Abdul Hameed <ahameed@redhat.com> --------- Signed-off-by: Abdul Hameed <ahameed@redhat.com>
lokeshrangineni · Sep 19, 2024 · 50b8f23 · 50b8f23
1 parent 0fb76e9
commit 50b8f23
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 12 deletions.
diff --git a/infra/charts/feast-feature-server/templates/deployment.yaml b/infra/charts/feast-feature-server/templates/deployment.yaml
@@ -42,20 +42,28 @@ spec:
           command:
             {{- if eq .Values.feast_mode "offline" }}
             - "feast"
+            - "--log-level"
+            - "{{ .Values.logLevel }}"
             - "serve_offline"
             - "-h"
             - "0.0.0.0"
             {{- else if eq .Values.feast_mode "ui" }}
             - "feast"
+            - "--log-level"
+            - "{{ .Values.logLevel }}"
             - "ui"
             - "-h"
             - "0.0.0.0"
             {{- else if eq .Values.feast_mode "registry" }}
             - "feast"
+            - "--log-level"
+            - "{{ .Values.logLevel }}"
             - "serve_registry"
             {{- else }}
             {{- if .Values.metrics.enlabled }}
             - "feast"
+            - "--log-level"
+            - "{{ .Values.logLevel }}"
             - "serve"
             - "--metrics"
             - "-h"

diff --git a/infra/charts/feast-feature-server/values.yaml b/infra/charts/feast-feature-server/values.yaml
@@ -11,6 +11,8 @@ image:
   # image.tag -- The Docker image tag (can be overwritten if custom feature server deps are needed for on demand transforms)
   tag: 0.40.0
 
+logLevel: "WARNING"  # Set log level DEBUG, INFO, WARNING, ERROR, and CRITICAL (case-insensitive)
+
 imagePullSecrets: []
 nameOverride: ""
 fullnameOverride: ""

diff --git a/sdk/python/feast/permissions/auth/kubernetes_token_parser.py b/sdk/python/feast/permissions/auth/kubernetes_token_parser.py
@@ -40,14 +40,16 @@ async def user_details_from_access_token(self, access_token: str) -> User:
         """
         sa_namespace, sa_name = _decode_token(access_token)
         current_user = f"{sa_namespace}:{sa_name}"
-        logging.info(f"Received request from {sa_name} in {sa_namespace}")
+        logger.info(
+            f"Request received from ServiceAccount: {sa_name} in namespace: {sa_namespace}"
+        )
 
         intra_communication_base64 = os.getenv("INTRA_COMMUNICATION_BASE64")
         if sa_name is not None and sa_name == intra_communication_base64:
             return User(username=sa_name, roles=[])
         else:
             roles = self.get_roles(sa_namespace, sa_name)
-            logging.info(f"SA roles are: {roles}")
+            logger.info(f"Roles for ServiceAccount {sa_name}: {roles}")
 
             return User(username=current_user, roles=roles)
 

diff --git a/sdk/python/feast/permissions/auth/oidc_token_parser.py b/sdk/python/feast/permissions/auth/oidc_token_parser.py
@@ -17,7 +17,6 @@
 from feast.permissions.user import User
 
 logger = logging.getLogger(__name__)
-logger.setLevel(logging.INFO)
 
 
 class OidcTokenParser(TokenParser):
@@ -69,8 +68,9 @@ async def user_details_from_access_token(self, access_token: str) -> User:
 
         try:
             await self._validate_token(access_token)
-            logger.info("Validated token")
+            logger.debug("Token successfully validated.")
         except Exception as e:
+            logger.error(f"Token validation failed: {e}")
             raise AuthenticationError(f"Invalid token: {e}")
 
         optional_custom_headers = {"User-agent": "custom-user-agent"}

diff --git a/sdk/python/feast/permissions/client/intra_comm_authentication_client_manager.py b/sdk/python/feast/permissions/client/intra_comm_authentication_client_manager.py
@@ -13,6 +13,7 @@ class IntraCommAuthClientManager(AuthenticationClientManager):
     def __init__(self, auth_config: AuthConfig, intra_communication_base64: str):
         self.auth_config = auth_config
         self.intra_communication_base64 = intra_communication_base64
+        logger.debug(f"AuthConfig type set to {self.auth_config.type}")
 
     def get_token(self):
         if self.auth_config.type == AuthType.OIDC.value:

diff --git a/sdk/python/feast/permissions/enforcer.py b/sdk/python/feast/permissions/enforcer.py
@@ -67,8 +67,12 @@ def enforce_policy(
                 if evaluator.is_decided():
                     grant, explanations = evaluator.grant()
                     if not grant and not filter_only:
+                        logger.error(f"Permission denied: {','.join(explanations)}")
                         raise FeastPermissionError(",".join(explanations))
                     if grant:
+                        logger.debug(
+                            f"Permission granted for {type(resource).__name__}:{resource.name}"
+                        )
                         _permitted_resources.append(resource)
                     break
         else:

diff --git a/sdk/python/feast/permissions/server/arrow.py b/sdk/python/feast/permissions/server/arrow.py
@@ -17,7 +17,6 @@
 from feast.permissions.user import User
 
 logger = logging.getLogger(__name__)
-logger.setLevel(logging.INFO)
 
 
 class AuthorizationMiddlewareFactory(fl.ServerMiddlewareFactory):
@@ -49,7 +48,9 @@ def __init__(self, access_token: str, *args, **kwargs):
 
     def call_completed(self, exception):
         if exception:
-            print(f"{AuthorizationMiddleware.__name__} received {exception}")
+            logger.exception(
+                f"{AuthorizationMiddleware.__name__} encountered an exception: {exception}"
+            )
 
     async def extract_user(self) -> User:
         """
@@ -69,14 +70,14 @@ def inject_user_details(context: ServerCallContext):
         context: The endpoint context.
     """
     if context.get_middleware("auth") is None:
-        logger.info("No `auth` middleware.")
+        logger.warning("No `auth` middleware.")
         return
 
     sm = get_security_manager()
     if sm is not None:
         auth_middleware = cast(AuthorizationMiddleware, context.get_middleware("auth"))
         current_user = asyncio.run(auth_middleware.extract_user())
-        print(f"extracted user: {current_user}")
+        logger.debug(f"User extracted: {current_user}")
 
         sm.set_current_user(current_user)
 

diff --git a/sdk/python/feast/permissions/server/grpc.py b/sdk/python/feast/permissions/server/grpc.py
@@ -9,7 +9,6 @@
 from feast.permissions.security_manager import get_security_manager
 
 logger = logging.getLogger(__name__)
-logger.setLevel(logging.INFO)
 
 
 class AuthInterceptor(grpc.ServerInterceptor):
@@ -22,11 +21,13 @@ def intercept_service(self, continuation, handler_call_details):
                 metadata=dict(handler_call_details.invocation_metadata)
             )
 
-            print(f"Fetching user for token: {len(access_token)}")
+            logger.debug(
+                f"Fetching user details for token of length: {len(access_token)}"
+            )
             current_user = asyncio.run(
                 auth_manager.token_parser.user_details_from_access_token(access_token)
             )
-            print(f"User is: {current_user}")
+            logger.debug(f"User is: {current_user}")
             sm.set_current_user(current_user)
 
         return continuation(handler_call_details)
diff --git a/sdk/python/feast/permissions/server/utils.py b/sdk/python/feast/permissions/server/utils.py
@@ -30,7 +30,6 @@
 from feast.permissions.server.rest_token_extractor import RestTokenExtractor
 
 logger = logging.getLogger(__name__)
-logger.setLevel(logging.INFO)
 
 
 class ServerType(enum.Enum):