-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[jaeger-v2] Map Collector Specific Flags To Corresponding OTEL Configurations #6040
Comments
After running
|
Most of these flags are for controlling server endpoints (which we should still document). The unique ones are these:
We should look how they map into various standard components in OTEL, like the batch processor, the attributes processor (which may not be even included in the binary yet). |
@yurishkuro couple of questions:
|
|
It's ok to say in the doc if there is no equivalent |
@yurishkuro Sounds good! What's the reason the first one is a no? And do you know of any equivalent to |
because Jaeger queue just keeps items in memory until they are processed, and it does not batch spans. It's actually not a great design for resiliency, it's better to have back pressure to the clients. |
got it! couple more questions:
|
There should also be a collector flag for concurrency (number of processors), we should find out what the equivalent for that is in OTEL. |
@yurishkuro I only see queues in the exporterhelper. Is this what we're looking for? |
@yurishkuro The migration document is mostly complete. Let me know if you have any feedback. I'm also looking for some guidance on how to proceed with |
I don't think we use exporter helper when exporting to storage - it's worth having a look into what features it would bring. |
Re num workers - I actually don't know what threading model OTEL uses, we should find out. |
@yurishkuro how would you like me to proceed regarding the above? would you like me to add some documentation about the exporter helper to the document? how can I find out what threading model OTEL uses? |
## Which problem is this PR solving? - Part of #6040 ## Description of the changes - Added the [attributesprocessor](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/attributesprocessor/README.md) to the Jaeger V2 binary to replace `--collector.tags`. See the [migration guide](https://docs.google.com/document/d/18B1yTMewRft2N0nW9K-ecVRTt5VaNgnrPTW1eL236t4/edit?usp=sharing) for more details. ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <[email protected]>
## Which problem is this PR solving? - Part of jaegertracing#6040 ## Description of the changes - Added the [attributesprocessor](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/attributesprocessor/README.md) to the Jaeger V2 binary to replace `--collector.tags`. See the [migration guide](https://docs.google.com/document/d/18B1yTMewRft2N0nW9K-ecVRTt5VaNgnrPTW1eL236t4/edit?usp=sharing) for more details. ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <[email protected]>
## Which problem is this PR solving? - Part of jaegertracing#6040 ## Description of the changes - Added the [attributesprocessor](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/attributesprocessor/README.md) to the Jaeger V2 binary to replace `--collector.tags`. See the [migration guide](https://docs.google.com/document/d/18B1yTMewRft2N0nW9K-ecVRTt5VaNgnrPTW1eL236t4/edit?usp=sharing) for more details. ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <[email protected]>
@yurishkuro any other action items here? looks like we still don't have a mapping for the following flags
Any thoughts on how to proceed? |
|
@yurishkuro we currently have queuing disabled here. The exporterhelper queue has a queue size field in the configuration (see https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/exporterhelper/internal/queue_sender.go#L34). Do we want to expose this in the storage exporter. There isn't anything here to configure memory though. |
yeah, I think we should enable all those features. Do they already have config structs exposed? |
@yurishkuro doesn't look like it - see https://github.com/jaegertracing/jaeger/blob/main/cmd/jaeger/internal/exporters/storageexporter/config.go#L17-L19. Should we embed the QueueConfig struct there? Also, are we not going to support the max memory? There's also the memorylimiterprocessor. |
@yurishkuro Opened at PR at #6080. |
no. It wasn't well supported in v1 either. |
## Which problem is this PR solving? - Towards #6040 ## Description of the changes - Added the `sending_queue` configuration to `jaeger_storage_exporter` from`exporterhelper` which will allow for enabling a queue when writing spans to any backend store. - This will allow for achieving parity in jaeger-v2 with the v1 collector's `--collector.queue-size` flag as `sending_queue` has a configuration for `queue_size`. - [Migration guide](https://docs.google.com/document/d/18B1yTMewRft2N0nW9K-ecVRTt5VaNgnrPTW1eL236t4/edit?usp=sharing) updated for the mapping between v1 and v2 ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <[email protected]>
@yurishkuro Any other items we want to tackle as part of this issue? If not, we can close it out. |
## Which problem is this PR solving? - Part of jaegertracing#6040 ## Description of the changes - Added the [attributesprocessor](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/attributesprocessor/README.md) to the Jaeger V2 binary to replace `--collector.tags`. See the [migration guide](https://docs.google.com/document/d/18B1yTMewRft2N0nW9K-ecVRTt5VaNgnrPTW1eL236t4/edit?usp=sharing) for more details. ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <[email protected]>
…tracing#6080) ## Which problem is this PR solving? - Towards jaegertracing#6040 ## Description of the changes - Added the `sending_queue` configuration to `jaeger_storage_exporter` from`exporterhelper` which will allow for enabling a queue when writing spans to any backend store. - This will allow for achieving parity in jaeger-v2 with the v1 collector's `--collector.queue-size` flag as `sending_queue` has a configuration for `queue_size`. - [Migration guide](https://docs.google.com/document/d/18B1yTMewRft2N0nW9K-ecVRTt5VaNgnrPTW1eL236t4/edit?usp=sharing) updated for the mapping between v1 and v2 ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <[email protected]>
Requirement
The text was updated successfully, but these errors were encountered: