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 details of Jaeger binary architecture #783

Merged
merged 8 commits into from
Nov 19, 2024

Conversation

jkowall
Copy link
Collaborator

@jkowall jkowall commented Nov 19, 2024

Copy link

netlify bot commented Nov 19, 2024

Deploy Preview for romantic-neumann-1959d7 ready!

Name Link
🔨 Latest commit c79b24d
🔍 Latest deploy log https://app.netlify.com/sites/romantic-neumann-1959d7/deploys/673cffb876843e0008326700
😎 Deploy Preview https://deploy-preview-783--romantic-neumann-1959d7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

It doesn't fully address the ticket, as I would like the diagram to be more detailed w.r.t. the core extensions

* [Jaeger](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/jaegerreceiver)

* [Kafka}](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/kafkareceiver)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* [Kafka}](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/kafkareceiver)
* [Kafka](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/kafkareceiver)


#### Connectors
* [Span Metrics](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/connector/spanmetricsconnector/)"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* [Span Metrics](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/connector/spanmetricsconnector/)"
* [Span Metrics](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/connector/spanmetricsconnector/)

content/docs/next-release-v2/architecture.md Outdated Show resolved Hide resolved
Comment on lines 87 to 88

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* [OTLP](https://github.com/open-telemetry/opentelemetry-collector/tree/main/receiver/otlpreceiver)

Aside from these components there are sevral other components from OpenTelemetry you can use in the config of the Jaeger binary. Here is the full list of components:

### Jaeger Components
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure we should be linking to the source code for these. We already have some documentation on the Configuration page https://deploy-preview-783--romantic-neumann-1959d7.netlify.app/docs/next-release-v2/configuration/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I still think it's good for users to get the context of the binary and where the sources come from.

* [Adaptive Sampling Processor](https://github.com/jaegertracing/jaeger/tree/main/cmd/jaeger/internal/processors/adaptivesampling)

* [Jaeger Storage Exporter](https://github.com/jaegertracing/jaeger/tree/main/cmd/jaeger/internal/extension/jaegerstorage)
Copy link
Member

Choose a reason for hiding this comment

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

also jaeger_storage extension, which is actually the 90% of Jaeger and define all storage backends. I would put it first on the list.

* [Jaeger Query Extension](https://github.com/jaegertracing/jaeger/tree/main/cmd/jaeger/internal/extension/jaegerquery)

* [Jaeger Remote Sampling Extension](https://github.com/jaegertracing/jaeger/tree/main/cmd/jaeger/internal/extension/remotesampling)
Copy link
Member

Choose a reason for hiding this comment

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

group adaptive sampling and remote sampling close together

![Architecture](/img/architecture-v2-binary.png)

Aside from these components there are sevral other components from OpenTelemetry you can use in the config of the Jaeger binary. Here is the full list of components:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Aside from these components there are sevral other components from OpenTelemetry you can use in the config of the Jaeger binary. Here is the full list of components:
Aside from these components there are several other components from OpenTelemetry you can use in the config of the Jaeger binary. Here is the full list of components:

content/docs/next-release-v2/architecture.md Show resolved Hide resolved
@jkowall
Copy link
Collaborator Author

jkowall commented Nov 19, 2024

It doesn't fully address the ticket, as I would like the diagram to be more detailed w.r.t. the core extensions

Can you provide what you mean by "core extensions" that you'd like to see?

@yurishkuro
Copy link
Member

Can you provide what you mean by "core extensions" that you'd like to see?

jaeger_storage, jaeger_query and jaeger_storage_exporter are what I consider core components, since without them there is no Jaeger-v2.

@jkowall
Copy link
Collaborator Author

jkowall commented Nov 19, 2024

All of these should be addressed with the latest push (two).

@yurishkuro
Copy link
Member

linter complains of missleppings

/home/runner/work/documentation/documentation/content/docs/next-release-v2/architecture.md:79:6 - Unknown word (Recievers) fix: (Receivers)
/home/runner/work/documentation/documentation/content/docs/next-release-v2/architecture.md:80:103 - Unknown word (Recieve) fix: (Receive)
/home/runner/work/documentation/documentation/content/docs/next-release-v2/architecture.md:82:115 - Unknown word (Recieve) fix: (Receive)
/home/runner/work/documentation/documentation/content/docs/next-release-v2/architecture.md:84:113 - Unknown word (Recieve) fix: (Receive)
/home/runner/work/documentation/documentation/content/docs/next-release-v2/architecture.md:86:115 - Unknown word (Recieve) fix: (Receive)
/home/runner/work/documentation/documentation/content/docs/next-release-v2/architecture.md:98:126 - Unknown word (OLTP)
/home/runner/work/documentation/documentation/content/docs/next-release-v2/architecture.md:100:1[18](https://github.com/jaegertracing/documentation/actions/runs/11921650283/job/33226240441?pr=783#step:4:19) - Unknown word (OLTP)

Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
@jkowall
Copy link
Collaborator Author

jkowall commented Nov 19, 2024

Should be fixed now, sorry for the missed spelling correction.

@yurishkuro yurishkuro merged commit 595c299 into jaegertracing:main Nov 19, 2024
10 checks passed
@jkowall jkowall deleted the arch branch November 19, 2024 22:27
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.

2 participants