Skip to content
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.

Sign up for GitHub

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

Add request response logging via fluentd #961

Merged
merged 11 commits into from
Sep 8, 2020
Merged

Add request response logging via fluentd #961

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
da4fea4
Add request response logging via fluentd
terryyylim Aug 24, 2020
bdafa3b
Address PR comments
terryyylim Aug 27, 2020
1f93df6
Update log forwarding logic
terryyylim Sep 2, 2020
3ae2fd2
Make application config clearer
terryyylim Sep 2, 2020
e89ef29
Address PR comments
terryyylim Sep 7, 2020
e630aef
Update constants
terryyylim Sep 8, 2020
10bed69
Rename constant
terryyylim Sep 8, 2020
c639dca
Use log insteadof throwing error
terryyylim Sep 8, 2020
a53d640
Update value retrieval
terryyylim Sep 8, 2020
6c660c5
Address PR comments
terryyylim Sep 8, 2020
afb4ed6
Remove log and fix message cast bug
terryyylim Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions common/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.fluentd</groupId>
<artifactId>fluent-logger</artifactId>
<version>0.3.1</version>
</dependency>

<!-- Bean Validation API support -->
<dependency>
Expand Down
76 changes: 58 additions & 18 deletions common/src/main/java/feast/common/logging/AuditLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package feast.common.logging;

import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.util.JsonFormat;
import feast.common.logging.config.LoggingProperties;
import feast.common.logging.config.LoggingProperties.AuditLogProperties;
import feast.common.logging.entry.ActionAuditLogEntry;
Expand All @@ -24,7 +26,11 @@
import feast.common.logging.entry.LogResource.ResourceType;
import feast.common.logging.entry.MessageAuditLogEntry;
import feast.common.logging.entry.TransitionAuditLogEntry;
import java.util.HashMap;
import java.util.Map;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.fluentd.logger.FluentLogger;
import org.slf4j.Marker;
import org.slf4j.MarkerFactory;
import org.slf4j.event.Level;
Expand All @@ -35,7 +41,10 @@
@Slf4j
@Component
public class AuditLogger {
private static final String FLUENTD_DESTINATION = "fluentd";
private static final String DEFAULT_RELEASE_NAME = "feast-0-0";
private static final Marker AUDIT_MARKER = MarkerFactory.getMarker("AUDIT_MARK");
private static FluentLogger fluentLogger;
private static AuditLogProperties properties;
private static BuildProperties buildProperties;

Expand All @@ -46,6 +55,14 @@ public AuditLogger(LoggingProperties loggingProperties, BuildProperties buildPro
// This allows us to use the dependencies in the AuditLogger's static methods
AuditLogger.properties = loggingProperties.getAudit();
AuditLogger.buildProperties = buildProperties;
if (AuditLogger.properties.getMessageLogging() != null
&& AuditLogger.properties.getMessageLogging().isEnabled()) {
AuditLogger.fluentLogger =
FluentLogger.getLogger(
"feast",
AuditLogger.properties.getMessageLogging().getFluentdHost(),
AuditLogger.properties.getMessageLogging().getFluentdPort());
}
}

/**
Expand Down Expand Up @@ -112,24 +129,47 @@ private static void log(Level level, AuditLogEntry entry) {
return;
}

// Log event to audit log through enabled formats
String entryJSON = entry.toJSON();
switch (level) {
case TRACE:
log.trace(AUDIT_MARKER, entryJSON);
break;
case DEBUG:
log.debug(AUDIT_MARKER, entryJSON);
break;
case INFO:
log.info(AUDIT_MARKER, entryJSON);
break;
case WARN:
log.warn(AUDIT_MARKER, entryJSON);
break;
case ERROR:
log.error(AUDIT_MARKER, entryJSON);
break;
// Either forward log to logging layer or log to console
if (properties.getMessageLogging().getDestination().equals(FLUENTD_DESTINATION)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What prevents this code from executing if message Logging is disabled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's handled here from #940.

Copy link
Member

@woop woop Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, as a convention we try and initialize variables from getters in order to make code more readable. So something like

String destination = properties.getMessageLogging().getDestination();
if (destination.equals(FLUENTD_DESTINATION)) {

}

Map<String, Object> fluentdLogs = new HashMap<>();
String releaseName =
StringUtils.defaultIfEmpty(System.getenv("RELEASE_NAME"), DEFAULT_RELEASE_NAME);
MessageAuditLogEntry messageAuditLogEntry = (MessageAuditLogEntry) entry;

fluentdLogs.put("id", messageAuditLogEntry.getId());
fluentdLogs.put("service", messageAuditLogEntry.getService());
fluentdLogs.put("status_code", messageAuditLogEntry.getStatusCode());
fluentdLogs.put("method", messageAuditLogEntry.getMethod());
fluentdLogs.put("release_name", releaseName);
try {
fluentdLogs.put("request", JsonFormat.printer().print(messageAuditLogEntry.getRequest()));
fluentdLogs.put("response", JsonFormat.printer().print(messageAuditLogEntry.getResponse()));
} catch (InvalidProtocolBufferException e) {
log.error(
"Request/Response log conversion to JSON failed. Unable to forward logs to logging service.",
e);
}
fluentLogger.log("fluentd", fluentdLogs);
} else {
// Log event to audit log through enabled formats
String entryJSON = entry.toJSON();
switch (level) {
case TRACE:
log.trace(AUDIT_MARKER, entryJSON);
break;
case DEBUG:
log.debug(AUDIT_MARKER, entryJSON);
break;
case INFO:
log.info(AUDIT_MARKER, entryJSON);
break;
case WARN:
log.warn(AUDIT_MARKER, entryJSON);
break;
case ERROR:
log.error(AUDIT_MARKER, entryJSON);
break;
}
}
}
}
19 changes: 17 additions & 2 deletions common/src/main/java/feast/common/logging/config/LoggingProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,22 @@ public static class AuditLogProperties {
// Whether to enable/disable audit logging entirely.
private boolean enabled;

// Whether to enable/disable message level (ie request/response) audit logging.
private boolean messageLoggingEnabled;
private MessageLogging messageLogging;

@Getter
@Setter
public static class MessageLogging {
// Whether to enable/disable message level (ie request/response) audit logging.
private boolean enabled;

// Whether to log to console or fluentd
private String destination;
woop marked this conversation as resolved.
Show resolved Hide resolved

// fluentD service host for external (request/response) logging.
private String fluentdHost;

// fluentD service port for external (request/response) logging.
private Integer fluentdPort;
}
}
}
2 changes: 1 addition & 1 deletion common/src/main/java/feast/common/logging/interceptors/GrpcMessageInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public GrpcMessageInterceptor(
public <ReqT, RespT> Listener<ReqT> interceptCall(
ServerCall<ReqT, RespT> call, Metadata headers, ServerCallHandler<ReqT, RespT> next) {
// Disable the message logging interceptor entirely if message logging is disabled.
if (!loggingProperties.getAudit().isMessageLoggingEnabled()) {
if (!loggingProperties.getAudit().getMessageLogging().isEnabled()) {
return next.startCall(call, headers);
}

Expand Down
9 changes: 8 additions & 1 deletion core/src/main/resources/application.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,14 @@ feast:
# Whether audit logging is enabled.
enabled: true
# Whether to enable message level (ie request/response) audit logging
messageLoggingEnabled: false
messageLogging:
enabled: false
# Logging forwarder currently provides a machine readable structured JSON log to an
# external fluentd service that can give better insight into what is happening in Feast.
# Accepts console / fluentd as destination
destination: console
fluentdHost: localhost
fluentdPort: 24224

grpc:
server:
Expand Down
3 changes: 2 additions & 1 deletion core/src/test/java/feast/core/logging/CoreLoggingIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
@SpringBootTest(
properties = {
"feast.logging.audit.enabled=true",
"feast.logging.audit.messageLoggingEnabled=true",
"feast.logging.audit.messageLogging.enabled=true",
"feast.logging.audit.messageLogging.destination=console"
})
public class CoreLoggingIT extends BaseIT {
private static TestLogAppender testAuditLogAppender;
Expand Down
9 changes: 8 additions & 1 deletion job-controller/src/main/resources/application.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,14 @@ feast:
# Whether audit logging is enabled.
enabled: true
# Whether to enable message level (ie request/response) audit logging
messageLoggingEnabled: false
messageLogging:
enabled: false
# Logging forwarder currently provides a machine readable structured JSON log to an
# external fluentd service that can give better insight into what is happening in Feast.
# Accepts console / fluentd as destination
destination: console
fluentdHost: localhost
fluentdPort: 24224

grpc:
server:
Expand Down
9 changes: 8 additions & 1 deletion serving/src/main/resources/application.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,14 @@ feast:
# Whether audit logging is enabled.
enabled: true
# Whether to enable message level (ie request/response) audit logging
messageLoggingEnabled: false
messageLogging:
enabled: false
# Logging forwarder currently provides a machine readable structured JSON log to an
# external fluentd service that can give better insight into what is happening in Feast.
# Accepts console / fluentd as destination
destination: console
fluentdHost: localhost
fluentdPort: 24224

grpc:
server:
Expand Down