search

cargomedia / cm-janus

UNMAINTAINED. cm/janus bridge
MIT License
2 stars 6 forks source link

Add session context to most log records #215

Closed njam closed 8 years ago

njam commented 8 years ago

Via https://github.com/cargomedia/puppet-cargomedia/issues/1084

@tomaszdurka @vogdb For debugging purposes it would be good to have a common field to identify the session of a log record. Preferably that session context could be enriched when new information arrives. Maybe we can even parse the json "token" and extract the "web session id" to add it as well?

let's discuss!

Depends on #186 cause of websocket server url parsing

njam commented 8 years ago

Allow to pass an arbitrary string identifier when connecting to cm-janus:

/janus?context={"clientId":"foo123","userId":123}

The fields in the context should be assigned to the WebSocket connection and included in all log messages relating to this connection.

How to make it available everywhere? Having a "context" object? We can enrich that object with janus concepts like "janusSessionId" etc.

njam commented 8 years ago

Let's do these steps:

  1. Add context for websocket events (to browser and to janus)
  2. Other events (cm http client, conversion jobs, bin/cm, http server)
vogdb commented 8 years ago

@tomaszdurka This is very funny! As I've mentioned once in our discussion to #186 that the ws lib, which we use for cm-janus websocket server, does not allow to access url of incoming connection. I offered a replacement to another websocket library but you was against it. Can I do the replacement now? Should I upgrade ws lib to have this functionality(create a custom fork of it)?

vogdb commented 8 years ago

@tomaszdurka @njam here is a short draft.

vogdb commented 8 years ago

@tomaszdurka @njam please compare the approaches @d68cd00 and @8057e33. What do you prefer more?

tomaszdurka commented 8 years ago

Closing in favor of #235 #236.