Glassdoor / planout4j

Java port of Facebook's PlanOut A/B testing system with additional functionality
Other
117 stars 50 forks source link

Design and implement pluggable exposure logging #23

Open eric239 opened 9 years ago

eric239 commented 9 years ago

While different consumers of p4j will likely have different logging implementations, it would be nice to have a pluggable framework that abstracts the "what" to log from "how" it gets logged

eric239 commented 9 years ago

Adding a bit more specifics.

First, let's define an interface for logging implementation:

public interface Planout4jLog {
  void log(Planout4jLogRecord r);
}

Default implementation will utilize SLF4J.

The log record class will encapsulate exposure information per namespace, including (but not limited to)

Let's also define a separate interface for serializing an instance of log record:

public interace Planout4jLogRecordSerializer {
  String serialize(LogRecord r);
}

Default implementation will produce JSON.

Each YAML namespace config will be extended to include the following logging-related properties:

reikje commented 9 years ago

Looks good I think. At which point are you planning to create the Planout4jLogRecord?

Example based on the simple config file. A user can either see the default button #000000 | Register or buttons that are permutations of #ff0000, #00ff00, Join now!, Sign up.

A client code example might look like:

NamespaceFactory nsFact = new SimpleNamespaceFactory(); // done once on startup?
Namespace ns = nsFact.getNamespace("test-ns", Collections.singletonMap("userid", 123).get(); // done for each user
String buttonText = ns.getParam("button_text", "default"); // can we populate a full Planout4jLogRecord here?
String buttonColor = ns.getParam("button_color", "default"); // or are the params evaluated lazy?

Does any of the calls above initiate a callback to the log method or will it be an extra (explicit) call?

eric239 commented 8 years ago

In the above example, the 2nd line will trigger logging. There's an implicit assumption that application will "cache" Namespace instance (which in turn has all the parameters computed and cached after creation, so any of getParam calls will not trigger logging)

BTW, the implementation of the above suggested framework is about to begin 😄

ivandd commented 8 years ago

@eric239 it's great that you have found time to do this. Looks like the log record has what one would normally want to log.

nkapur commented 7 years ago

Quick question on this: I see 1.2 was released in May 2016 - so I'm assuming these changes did not make it in yet. Is there a plan to release a 1.3?

eric239 commented 7 years ago

@nkapur this project would benefit from a new maintainer(s). I (the original maintainer) have moved on and don't use planout4j in any of my current work.

@ryanaylward is there someone at Glassdoor interested in "adopting" planout4j?

nkapur commented 7 years ago

I think having configurable Logging would be a good stopping point. I believe active maintenance might not be required medium or long term.

We at GoFundMe are building components for venture projects without having to rely on the internal planout micro-service.