Skip to content

Conversation

@SolidWallOfCode
Copy link
Member

No description provided.

@SolidWallOfCode
Copy link
Member Author

I thought I saw a comment from James but it doesn't seem to be there now.

  1. I'm open to using a term other than "alert", if there are any specific suggestions.

  2. I will be working on updating the P.R. with additional example code of the utility of this mechanism which will also illustrate use cases.

@jpeach
Copy link
Contributor

jpeach commented Nov 23, 2015

I thought I saw a comment from James but it doesn't seem to be there now.

I think I made them on the JIRA.

  1. I'm open to using a term other than "alert", if there are any specific suggestions.

We should definitely not overload "alert" here. I think "message" is sufficient.

  1. I will be working on updating the P.R. with additional example code of the utility of this mechanism which will also illustrate use cases.

Great, looking forward to that!

Also, I think that the default mode of this feature should be to send a message to a specific, named plugin. Maybe you could optionally broadcast the message to all plugins, but it seems most useful to message a specific plugin.

The message itself should be a opaque bag of bytes. This would let you send things like serialize protobuf messages (for example).

@bgaff
Copy link
Member

bgaff commented Nov 25, 2015

I think this is something that will be incredibly useful, thanks for working on it.

@SolidWallOfCode
Copy link
Member Author

I thought quite a bit about broadcast vs. targeted messages and decided I prefer this style. It's based on my previous experience with message bus style mechanisms. The ability to impose additional structure on the messages is very useful and should be preserved. For instance a plugin might "subscribe" to more than one channel and use a common library to select messages based on those subscriptions. If you survey the standard kind of mechanism used for such message busses you will see this pattern (particularly using domain style names "a.b.c.d") as very common and I want that to be easy to implement on top of this mechanism. In terms of overall processing the same work will be done whether it's the core doing the name comparison or the plugin. I may want to add some additional support for this so a plugin can easily listen for messages with a specific prefix.

@jpeach
Copy link
Contributor

jpeach commented Nov 25, 2015

That seems fine, but this mechanism doesn't provide channels or any ability for plugins to subscribe to messages. There's no reason to think that all plugins will implement the same protocol on top of this mechanism, so preserving the ability to send messages to specific plugins still seems appropriate.

It sounds like you have a particular message format in mind. Could you clarify that?

… to "message". Made messaging a privileged operation.
@SolidWallOfCode
Copy link
Member Author

I have updated the PR with several changes.

  • "Alert" is now "Message"
  • Messaging is a privileged operation.
  • Messages have a tag and payload (data). The former is a null terminated string and the latter is a slab of bytes.
  • traffic_ctl requires both tag and data to be null terminated strings because they are both command line arguments.

I haven't had a chance to work on example uses yet, although I have made progress in designing them.

@jpeach
Copy link
Contributor

jpeach commented Dec 1, 2015

Looks nice. You don't need to hand-marshall the local manager message. Just use mgmt_message_length() and mgmt_message_marshall() to marshall into a buffer and send that as the payload of signalEvent.

I think the changes to the public API should go through API review?

@SolidWallOfCode
Copy link
Member Author

I hand marshall the message from traffic_manager to traffic_server because the technique you suggest did not work for me. What I did was pass the raw message data, which should be the same thing as you suggested (because the raw data is the marshalled message from traffic_ctl). But I saw no way to unmarshall the data when it arrives in traffic_server. If you know how to do that, feel free to fix this after it's committed.

@SolidWallOfCode
Copy link
Member Author

I need to work on the documentation a bit more before I send it out for formal API review. I would also like to build better example / use case code. It might well be that in that process I tweak the API to better suit actual use.

@jpeach
Copy link
Contributor

jpeach commented Dec 1, 2015

Message marshaling works just fine, see #301 for an example of exactly what you want to do here. If you can show me the problem then I can try to help.

@bryancall
Copy link
Contributor

@SolidWallOfCode and @dragon512 are going to take ownership of this.

@SolidWallOfCode
Copy link
Member Author

I changed the RPC unpacking to use the standard mechanism. I don't know why it didn't compile before and I don't the energy to figure it out.

SolidWallOfCode pushed a commit to SolidWallOfCode/trafficserver that referenced this pull request Feb 1, 2017
TS-5047: Fix unmapped URL logging tags.
SolidWallOfCode pushed a commit to SolidWallOfCode/trafficserver that referenced this pull request May 10, 2021
(cherry picked from commit cb4ff10)

Co-authored-by: thebadpete <peter.cheung@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants