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.

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

Provide utility for creating/updating event maps (e.g. via a JSON string) #30

Open
Geeber opened this issue Dec 1, 2014 · 11 comments
Open

Comments

@Geeber
Copy link
Contributor

Geeber commented Dec 1, 2014

Creating deeply nested maps in Java is kind of painful (see https://news.ycombinator.com/item?id=8637447). We should provide a utility to make it easier. One possible way (which has been explicitly requested at least once or twice) would be a method like:

Map<String, Object> buildEventFromJson(String json)

There are probably other good solutions.

@joshed-io
Copy link

I think it'd be good to call out a few popular Map builders in the docs, e.g. google commons ImmutableMap. That way people are aware of this type of tool, but we don't have to pick one, declare it as a hard dependency, wrap it, etc...

@mikereinhold
Copy link
Contributor

I've created some utility methods for converting "flat" maps to nested maps for use by the KeenClient (included in part of my current project, see here: https://github.com/CourseScheduler/scheduler-legacy/blob/master/src/main/java/io/devyse/scheduler/analytics/keen/KeenUtils.java).

I can submit this as an addition to KeenUtils or wherever is appropriate if you have interest.

@joshed-io
Copy link

Very cool!

I think this would make sense to include directly in the SDK only if a) flat map conversion is common to many SDK users and b) good documentation of how to use a map builder would still not address this need. @mikereinhold @Geeber what do you think? Should this capability be brought into the SDK, or is it something we talk about or link to from the docs?

@mikereinhold
Copy link
Contributor

My personal perspective is that, since Keen expects a nested map, it would make sense to include some form of conversion method or class that provides an automatically nested map.

That being said, if you do not want to bring the functionality into SDK directly, I can break the code out separate from the rest of my application so that it can be linked to and used by anyone.

@Geeber
Copy link
Contributor Author

Geeber commented Dec 8, 2014

What would you guys think about creating a separate event utilities jar? We could add it as another project in this repo and publish it in the same maven group, but as a separate artifact. That way folks wouldn't need to include it if they didn't want to, but it would be available and very seamless to integrate in.

@mikereinhold
Copy link
Contributor

Also fine with me.

What are your thoughts on creating a custom Map implementation that is a
NestedMap instead of just using nested semantics within an existing
Collections Map class?

On Mon, Dec 8, 2014 at 4:48 PM, Kevin Litwack [email protected]
wrote:

What would you guys think about creating a separate event utilities jar?
We could add it as another project in this repo and publish it in the same
maven group, but as a separate artifact. That way folks wouldn't need to
include it if they didn't want to, but it would be available and very
seamless to integrate in.


Reply to this email directly or view it on GitHub
#30 (comment)
.

@Geeber
Copy link
Contributor Author

Geeber commented Dec 8, 2014

Definitely open to that sort of thing :)

In the 3.0 changes, we could consider accepting some interface besides a Map for the events... something like:

Set<String[]> getKeys();
Object getValue(String[] keyPath);

Kind of brainstorming here. There are downsides but it would open up some interesting possibilities.

@Geeber
Copy link
Contributor Author

Geeber commented Dec 8, 2014

Actually I suppose we could always have the utility library provide an interface like that for writing, but still implement the plain old Map interface for reading. That's probably the best of both worlds. It'll take a bit of effort to make the implementation efficient but it should be doable.

@mikereinhold
Copy link
Contributor

Interesting ideas! I think there are definitely some options there that
could make things easier / simpler for the user of the SDK, specifically
around the requirements for the Map.

Implementing some conversion utilities for 2.x and gathering community
feedback would help to shape the design for 3.0.

On Mon, Dec 8, 2014 at 5:36 PM, Kevin Litwack [email protected]
wrote:

Actually I suppose we could always have the utility library provide an
interface like that for writing, but still implement the plain old Map
interface for reading. That's probably the best of both worlds. It'll take
a bit of effort to make the implementation efficient but it should be
doable.


Reply to this email directly or view it on GitHub
#30 (comment)
.

@pgoldweic
Copy link

As a new user, I'm interested in the status of this discussion. Has the utility for creating nested maps been incorporated in any way to this sdk or companion products? I personally think that conversion utilities should be included, given that a Map of that sort is required.
In addition, I wonder whether you would consider accepting a json string itself as an 'event' parameter, which would definitely simplify usage of this sdk (no need for conversions to maps). As a counterpart to this, I would probably also expect that an extraction query could return a json string as a result.

@Geeber
Copy link
Contributor Author

Geeber commented Oct 14, 2015

Unfortunately we haven't yet had a chance to formalize any utilities around this into something usable. It's still definitely on our radar but it's just been a question of engineering priorities - we haven't had the time. If anyone in the community wants to tackle this as an open source contribution, I'd be happy to work with them to make that happen ;)

As for KeenClient itself accepting a JSON string instead of a map, it's an appealing feature to have in principle but there are a couple downsides in practice:

  • If we want to still provide the option of using a Map, then we'd have to have overrides of every method with both options. This would substantially increase the number of public methods exposed by KeenClient, and there are already more than I'd like (3 each of addEvent, addEventAsync, and queueEvent).
  • If the user specified a JSON string, we would have to parse that JSON into a Map anyway so that we could add the keen.timestamp property, then re-serialize it back to JSON. This isn't horrible, since we do already have the KeenJsonHandler interface to handle the parsing, but it's somewhat inefficient.

So I would still kind of lean towards having the client take a Map (or some custom structured interface) and having a set of utilities to help build those Maps/interfaces, including potentially from a raw JSON string.

Your feedback is appreciated though, as it helps us understand what people would find helpful in practice. I'd definitely like to make it easier for users of the SDK to work with JSON directly, if that's what they prefer. I can't make any promises on timelines for that, but sooner or later we'll get there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants