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

add collector support activemq #2466

Closed
wants to merge 16 commits into from

Conversation

tian-junwei
Copy link
Contributor

like collector-rabbitmq the collector-activemq support collector consumes a ActiveMQ queue for messages that contain a list of spans

@codefromthecrypt
Copy link
Member

@IAMTJW I like your energy and eagerness to do work. We have a change policy though which suggests multiple people need to buy-in before we make a standard feature. For example, adding this here also has impact as people using tracers will also want it. there's a lot of work around it.

I see that there are a couple votes here which is good. #1990

We do need buy-in from folks in @openzipkin/core before adding this. Let's try to see if anyone is interested. Whatever we add becomes years of support work, so it is only fair to make sure people are ok with that.

@tian-junwei
Copy link
Contributor Author

@IAMTJW I like your energy and eagerness to do work. We have a change policy though which suggests multiple people need to buy-in before we make a standard feature. For example, adding this here also has impact as people using tracers will also want it. there's a lot of work around it.

I see that there are a couple votes here which is good. #1990

We do need buy-in from folks in @openzipkin/core before adding this. Let's try to see if anyone is interested. Whatever we add becomes years of support work, so it is only fair to make sure people are ok with that.

get it , our company use activemq as collector,use postgresql as storage and I want to make it available to others.

@codefromthecrypt
Copy link
Member

@IAMTJW would you be willing to contribute senders in zipkin-reporter-java and other languages for this?

@@ -0,0 +1,221 @@
zipkin:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need this file, I think you want to make activemq.yml right?

Collector collector = builder.delegate.build();
CollectorMetrics metrics = builder.metrics;
messageConsumer.setMessageListener(new ActiveMQSpanConsumerMessageListener(collector,metrics));
}catch (Exception e){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use multi-catch for what is actually thrown, unless literally Exception is thrown

Copy link
Member

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is good progress. please look at tests for rabbit or kafka to ensure yours are equally comprehensive.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im ready

@codefromthecrypt
Copy link
Member

@IAMTJW @moekelley76 can you put your vote on the original issue with thumbsup? #1990

@tian-junwei
Copy link
Contributor Author

this is good progress. please look at tests for rabbit or kafka to ensure yours are equally comprehensive.

Is there a activemq server to be used test ?

@jbertram
Copy link

How important is performance here, and what's the performance like for this implementation? ActiveMQ 5.x isn't particularly high-performance at this point given its underlying architecture, and storing messages in a database further slows things down. I'm new to Zipkin so I'm curious. Thanks!

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Mar 26, 2019 via email

@jbertram
Copy link

I was asking about performance more generally and also as it applies to this particular implementation. Is the performance of a collector in general an important factor? Assuming this collector is "applicable," what kind of performance does it provide?

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Mar 26, 2019 via email

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Mar 26, 2019 via email

@jbertram
Copy link

Is a few hundred messages per second considered near the top-end of the potential volume?

@codefromthecrypt
Copy link
Member

@jbertram depending on bundling mechanics, higher throughput can be thousands or tens of thousands per second. Higher throughput sites often run multiple collectors though. I would say that most users will not be doing more than a thousand of messages/second.

In the future, this may change as more people explore the costs involved in unsampled (usually this means late sampled, or tiered) collection. For example, we have a https://github.com/adriancole/zipkin-voltdb experiment which can swallow a lot more data than most storage backends. If someone doing 1k messages per second at 1% decides to do 100%, that means multiplying the throughput. For example, some doing what we call "firehose mode" like yelp, they had to write custom collectors as they went over what their kafka setup could do. This is more a thought for you vs what's commonly the case now. I mean to hint there could be some sites doing 100k messages/second into a short term buffer, even if I have no idea when that would be practiced and what the sharding of that rate would typically look like.

Back to reality :)

You'll notice some mentioning the order of thousands of spans per second here. No one currently is using activemq, but that's what this issue is about :) spans should be bundled (ex many spans per message), so if you have say.. 8k spans/second, ideally this is bundled into <1k messages/second of size in orders of low kilobytes.

https://cwiki.apache.org/confluence/display/ZIPKIN/Sites

I'll ask some of the sites to provide more fine grained details on bundling mechanics. Thanks very much for the interest!

@codefromthecrypt
Copy link
Member

@IAMTJW are you using this in production now (from a fork?) if so, what's your ingest rate?

PS it may be useful to bring this work into a separate repo to evaluate. for example, I made a project like this to help people test/create mongo without affecting the core repository. I can make something like this for you if it would help. https://github.com/adriancole/zipkin-mongodb

@tian-junwei
Copy link
Contributor Author

@IAMTJW are you using this in production now (from a fork?) if so, what's your ingest rate?

PS it may be useful to bring this work into a separate repo to evaluate. for example, I made a project like this to help people test/create mongo without affecting the core repository. I can make something like this for you if it would help. https://github.com/adriancole/zipkin-mongodb

We are using this in production(release 2.8.3), but we have only dozens of applications and have not much spans per second here

@codefromthecrypt
Copy link
Member

@moekelley76 by "I'm ready" does this mean you can try this code? what are you using now and how much throughput of messages do you expect?

@codefromthecrypt
Copy link
Member

@ImFlog PS we are still waiting for another person to mention they will test this.

As this repository will shortly move to the Apache Software Foundation (ASF) and this is non-trivial code, you should probably prepare by sending a contributor agreement to the ASF. If you have already done this in the past, no work to do. This is a once in life thing.

If you haven't already, easiest way is to download the ICLA template. After filling the icla.pdf with personal information correctly, print, sign, scan, and send it in mail as an attachment to the secretary ([email protected])

@codefromthecrypt
Copy link
Member

whoops I meant @IAMTJW not @ImFlog !

@tian-junwei
Copy link
Contributor Author

whoops I meant @IAMTJW not @ImFlog !

I have send mail to [email protected]

@codefromthecrypt
Copy link
Member

PS this repo is no longer moving to ASF (none are)

@thanhctES
Copy link

I am waiting for this. Please merge it.

codefromthecrypt pushed a commit that referenced this pull request Jun 23, 2019
Due to popular demand, this adds support for ActiveMQ 5.x.
This is enabled when the env variable `ACTIVEMQ_URL` is set to a valid
broker. Thanks very much to @IAMTJW for early work towards this change.

To try this change, you can use jitpack https://jitpack.io/#openzipkin/zipkin

Ex.
```bash
TAG=activemq-SNAPSHOT
curl -sSL https://jitpack.io/com/github/openzipkin/zipkin/zipkin-server/${TAG}/zipkin-server-${TAG}-exec.jar > zipkin.jar
ACTIVEMQ_URL=tcp://localhost:61616 java -jar zipkin.jar
```

Supercedes #2466
Fixes #1990
@codefromthecrypt
Copy link
Member

superceded by #2639

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