Skip to content

Conversation

oiooj
Copy link
Contributor

@oiooj oiooj commented Feb 8, 2018

The ProcessorConfiguration can be exported, but the custom type model and protocol can not, so it's hard to new a ProcessorConfiguration outside the package.

Signed-off-by: kun oiooj@qq.com

@black-adder
Copy link
Contributor

May I know the use case for this? Are you planning on writing an entirely new span model?

@oiooj
Copy link
Contributor Author

oiooj commented Feb 8, 2018

@black-adder We want merge jaeger agent into our agent, so that we can manage only one agent on all servers.

@oiooj
Copy link
Contributor Author

oiooj commented Feb 9, 2018

@black-adder Can this be merged?

@black-adder
Copy link
Contributor

Sure, some tests are failing though:

cmd/agent/app/builder_test.go:159:15: undefined: model
cmd/agent/app/builder_test.go:160:15: undefined: protocol

@oiooj
Copy link
Contributor Author

oiooj commented Feb 9, 2018

Thank you, I will fix it tomorrow morning

The `ProcessorConfiguration` can be exported, but the custom type
`model` and `protocol` can not, so it's hard to new a
`ProcessorConfiguration` outside the pacaage.

Signed-off-by: kun <oiooj@qq.com>
@oiooj
Copy link
Contributor Author

oiooj commented Feb 10, 2018

@black-adder updated

@coveralls
Copy link

coveralls commented Feb 10, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 63ce7d2 on oiooj:pr-type into ef7a367 on jaegertracing:master.

@ghost ghost assigned black-adder Feb 12, 2018
@ghost ghost added the review label Feb 12, 2018
@black-adder black-adder merged commit 9fe8cc8 into jaegertracing:master Feb 12, 2018
@ghost ghost removed the review label Feb 12, 2018
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.

3 participants