Old Code, New Ideas

Looking back at my old code with bemusement and horror.

About 3 years ago I put a lot of effort into a set of golang packages called mediocre-go-lib. The idea was to create a framework around the ideas I had laid out in this blog post around the structure and composability of programs. What I found in using the framework was that it was quite bulky, not fully thought out, and ultimately difficult for anyone but me to use. So…. a typical framework then.

My ideas about program structure haven’t changed a ton since then, but my ideas around the patterns which enable that structure have simplified dramatically (see my more recent post for more on that). So in that spirit I’ve decided to cut a v2 branch of mediocre-go-lib and start trimming the fat.

This is going to be an exercise both in deleting old code (very fun) and re-examining old code which I used to think was good but now know is bad (even more fun), and I’ve been looking forward to it for some time.

mcmp, mctx

The two foundational pieces of mediocre-go-lib are the mcmp and mctx packages. mcmp primarily deals with its mcmp.Component type, which is a key/value store which can be used by other packages to store and retrieve component-level information. Each mcmp.Component exists as a node in a tree of mcmp.Components, and these form the structure of a program. mcmp.Component is able to provide information about its place in that tree as well (i.e. its path, parents, children, etc…).

If this sounds cumbersome and of questionable utility that’s because it is. It’s also not even correct, because a component in a program exists in a DAG, not a tree. Moreover, each component can keep track of whatever data it needs for itself using typed fields on a struct. Pretty much all other packages in mediocre-go-lib depend on mcmp to function, but they don’t need to, I just designed it that way.

So my plan of attack is going to be to delete mcmp completely, and repair all the other packages.

The other foundational piece of mediocre-go-lib is mctx. Where mcmp dealt with arbitrary key/value storage on the component level, mctx deals with it on the contextual level, where each go-routine (i.e. thread) corresponds to a context.Context. The primary function of mctx is this one:

// Annotate takes in one or more key/value pairs (kvs' length must be even) and
// returns a Context carrying them.
func Annotate(ctx context.Context, kvs ...interface{}) context.Context

I’m inclined to keep this around for now because it will be useful for logging, but there’s one change I’d like to make to it. In its current form the value of every key/value pair must already exist before being used to annotate the context.Context, but this can be cumbersome in cases where the data you’d want to annotate is quite hefty to generate but also not necessarily going to be used. I’d like to have the option to make annotating occur lazily. For this I add an Annotator interface and a WithAnnotator function which takes it as an argument, as well as some internal refactoring to make it all work right:

// Annotations is a set of key/value pairs representing a set of annotations. It
// implements the Annotator interface along with other useful post-processing
// methods.
type Annotations map[interface{}]interface{}

// Annotator is a type which can add annotation data to an existing set of
// annotations. The Annotate method should be expected to be called in a
// non-thread-safe manner.
type Annotator interface {
	Annotate(Annotations)
}

// WithAnnotator takes in an Annotator and returns a Context which will produce
// that Annotator's annotations when the Annotations function is called. The
// Annotator will be not be evaluated until the first call to Annotations.
func WithAnnotator(ctx context.Context, annotator Annotator) context.Context

Annotator is designed like it is for two reasons. The more obvious design, where the method has no arguments and returns a map, would cause a memory allocation on every invocation, which could be a drag for long chains of contexts whose annotations are being evaluated frequently. The obvious design also leaves open questions about whether the returned map can be modified by whoever receives it. The design given here dodges these problems without any obvious drawbacks.

The original implementation also had this unnecessary Annotation type:

// Annotation describes the annotation of a key/value pair made on a Context via
// the Annotate call.
type Annotation struct {
       Key, Value interface{}
}

I don’t know why this was ever needed, as an Annotation was never passed into nor returned from any function. It was part of the type AnnotationSet, but that could easily be refactored into a map[interface{}]interface{} instead. So I factored Annotation out completely.

mcfg, mrun

The next package to tackle is mcfg, which deals with configuration via command line arguments and environment variables. The package is set up to use the old mcmp.Component type such that each component could declare its own configuration parameters in the global configuration. In this way the configuration would have a hierarchy of its own which matches the component tree.

Given that I now think mcmp.Component isn’t the right course of action it would be the natural step to take that aspect out of mcfg, leaving only a basic command-line and environment variable parser. There are many other basic parsers of this sort out there, including one or two I wrote myself, and frankly I don’t think the world needs another. So mcfg is going away.

The mrun package is the corresponding package to mcfg; where mcfg dealt with configuration of components mrun deals with the initialization and shutdown of those same components. Like mcfg, mrun relies heavily on mcmp.Component, and doesn’t really have any function with that type gone. So mrun is a gonner too.

mlog

The mlog package is primarily concerned with, as you might guess, logging. While there are many useful logging packages out there none of them integrate with mctx’s annotations, so it is useful to have a custom logging package here. mlog also has the nice property of not being extremely coupled to mcmp.Component like other packages; it’s only necessary to delete a handful of global functions which aren’t a direct part of the mlog.Logger type in order to free the package from that burden.

With that said, the mlog.Logger type could still use some work. It’s primary pattern looks like this:

// Message describes a message to be logged.
type Message struct {
	Level
	Description string
	Contexts []context.Context
}

// Info logs an InfoLevel message.
func (l *Logger) Info(descr string, ctxs ...context.Context) {
	l.Log(mkMsg(InfoLevel, descr, ctxs...))
}

The idea was that if the user has multiple Contexts in hand, each one possibly having some relevant annotations, all of those Contexts’ annotations could be merged together for the log entry.

Looking back it seems to me that the only thing mlog should care about is the annotations, and not where those annotations came from. So the new pattern looks like this:

// Message describes a message to be logged.
type Message struct {
	Context context.Context
	Level
	Description string
	Annotators  []Annotators
}

// Info logs a LevelInfo message.
func (l *Logger) Info(ctx context.Context, descr string, annotators ...mctx.Annotator)

The annotations on the given Context will be included, and then any further Annotators can be added on. This will leave room for merr later.

There’s some other warts in mlog.Logger that should be dealt with as well, including some extraneous methods which were only used due to mcmp.Component, some poorly named types, a message handler which didn’t properly clean itself up, and making NewLogger take in parameters with which it can be customized as needed (previously it only allowed for a single configuration). I’ve also extended Message to include a timestamp, a namespace field, and some other useful information.

Future Work

I’ve run out of time for today, but future work on this package includes:

  • Updating merr with support for mctx.Annotations.
  • Auditing the mnet, mhttp, and mrpc packages to see if they contain anything worth keeping.
  • Probably deleting the m package entirely; I don’t even really remember what it does.
  • Probably deleting the mdb package entirely; it only makes sense in the context of mcmp.Component.
  • Making a difficult decision about mtest; I put a lot of work into it, but is it really any better than testify?