Refactored Zipkin Proxy artifacts #33
Conversation
saturnism
commented
Mar 24, 2017
•
saturnism
commented
- moved EnableStackdriverCollector to the autoconfigure module
- created a starter pom.xml so that others can build their own collector (and override things like Credentials easily)
- changed the resolution of projectId property
- configured dependency versions via dependency management
- created a starter pom.xml so that others can build their own collector (and override things like Credentials easily) - changed the resolution of projectId property
|
Rebased to the latest commits. Previous one had too many merge conflicts. from @adriancole #32
The primary needs here is to be able to override the production of Credential. In different env, we need to create this differently.
I noticed autoconfigure-storage was pulled into its own artifact. Is this what you are referring to? That could help. But
|
|
/cc @joshlong |
|
The primary needs here is to be able to override the production of
Credential. In different env, we need to create this differently.
Let's take STS for example. We document the means by which credentials can
be found, ex from the environment (ex metadata service) or from properties
(mapped to env variables).
https://github.com/openzipkin/zipkin-aws/tree/master/autoconfigure/collector-sqs#security
Via this, users have been able to make no changes to the server and support
local and cloud-based credentials. This approach has worked well enough to
cause result in no issues requesting credential customization on the amazon
side (and is similar to the older aws elasticsearch storage component which
predated our STS support)
At any rate, an auto-configure jar can be placed in the classpath of the
server which can override whatever default is built-in. What sort of
credential supply motivated this change? maybe it is better to start with
that?
I noticed autoconfigure-storage was pulled into its own artifact. Is this
what you are referring to? That could help. But EnableStackdriverCollector
is still in collector. What's the use there?
I guess what I meant here is that if refactoring is done, there's another
way to do it which doesn't require a custom server at all. That is to
emulate what we do for aws and azure, where the storage module is
self-contained and can be added to the stock zipkin server in the same
fashion.
Ex. this build results in a self-contained jar that can be added to a
default zipkin server
https://github.com/openzipkin/zipkin-azure/blob/master/autoconfigure/collector-eventhub/pom.xml
we could consider something similar, where google support is the same
integration pattern as aws and azure, as opposed to a custom server build
which is enabled with an annotation.
If we want to bring the tomcat changes needed upstream to support that,
seems great to me..
Not sure what Tomcat is referred to.
I meant the stuff moved from here
https://github.com/GoogleCloudPlatform/stackdriver-zipkin/blob/master/collector/pom.xml#L19
The 'collector' wouldn't need to exist if there were no changes needed to
the upstream server (like the boringssl patch). It could still exist, as a
convenience, but it wouldn't have to.
If this project was refactored like the aws and azure ones were, you could
add stackdriver to a project that supported kafka for example, and not
require a custom build to do that. If one of the motivations for not using
the stock server was the boringssl patch, I'm mentioning that change could
go upstream with probably no resistance.
|
| import org.springframework.context.annotation.Import; | ||
| import zipkin.server.ZipkinHttpCollector; | ||
| import zipkin.server.ZipkinServerConfiguration; | ||
|
|
||
| import java.lang.annotation.*; |
There was a problem hiding this comment.
please rollback non-essential change
|
hiya sorry been pretty busy. not following most of the comments since the formatting seems off. I guess the easier way is - what do you recommend so that we can override the creation of the Credential object? https://github.com/GoogleCloudPlatform/stackdriver-zipkin/blob/master/autoconfigure-storage/src/main/java/com/google/cloud/trace/zipkin/autoconfigure/ZipkinStackdriverStorageAutoConfiguration.java#L85 Perhaps a sample project would help. We speak of a stock server - but I think our Zipkin server is pretty much non-stock, w/ our own Secondly, injecting a String for ProjectID seemed not Spring'ish and should probably be moved into properties? |
|
As far as I understand, the primary deliverable of this project is a
server, which is similar to how other zipkin servers are delivered. Custom
servers are usually surprising and sometimes also mistakes (ex in zipkin we
used to get a lot of unnecessarily custom servers due to start.spring.io
which has since been removed). For example, the majority of users do not
need to even know how a project id is supplied as they use variables
similarly to how the upstream project works
https://github.com/GoogleCloudPlatform/stackdriver-zipkin#configuration
When you say you want to override credentials, what type of credential
supply do you need to override with that isnt already supported via env? It
might be the case that a greater set of users can be served simply by
making that configurable without a custom build.
Custom builds when not necessary do create distraction in practice. For
example, it ends up with blogs where people explain which library versions
do what, or gitter questions on eclipse or other things. Usually we find
that these requestors were simply steered incorrectly that they need a
custom build.
Instead of asking for implementation, do you mind asking for use case? It
would help guide this in a way that doesn't cause more problems than it
solves.
…On Thu, Apr 6, 2017 at 8:34 AM, Ray Tsang ***@***.***> wrote:
hiya sorry been pretty busy. not following most of the comments since the
formatting seems off. I guess the easier way is - what do you recommend so
that we can override the creation of the Credential object?
https://github.com/GoogleCloudPlatform/stackdriver-zipkin/blob/
master/autoconfigure-storage/src/main/java/com/google/cloud/trace/zipkin/
autoconfigure/ZipkinStackdriverStorageAutoConfiguration.java#L85
Secondly, injecting a String for ProjectID seemed not Spring'ish and
should probably be moved into properties?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAD615sDTbpjKEmHIxBbBmWiGEzC7u77ks5rtDMjgaJpZM4Mnkuu>
.
|
|
here's an alternative which fits into the ecosystem well. I've discussed this with others in the spring cloud team as well #36 |
|
How can I help to get this merged? |
|
@bogdandrutu so we have a way to bolt-on things like this on existing servers (which allow easier custom images whether that's docker or chef or whatever) just needs focus to do it |
|
fyi haven't forgotten about this once #44 is in, I believe we can convert the collector to a normal plugin (aka module) |
|
Is this still needed? |
|
this will be done differently once other things are merged, so let's close it? |
