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

Replace command line args in deployment doc #751

Merged
merged 3 commits into from
Nov 2, 2024

Conversation

jkowall
Copy link
Collaborator

@jkowall jkowall commented Oct 2, 2024

Which problem is this PR solving?

Part of #750

Description of the changes

Adjusting command line and other components from v1 to v2.

Checklist

Copy link

netlify bot commented Oct 2, 2024

Deploy Preview for romantic-neumann-1959d7 ready!

Name Link
🔨 Latest commit f88c581
🔍 Latest deploy log https://app.netlify.com/sites/romantic-neumann-1959d7/deploys/67257e497324a400087ecc41
😎 Deploy Preview https://deploy-preview-751--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.

-p5778:5778/tcp \
jaegertracing/jaeger-agent:{{< currentVersion >}} \
--reporter.grpc.host-port=jaeger-collector.jaeger-infra.svc:14250
jaeger_storage:
Copy link
Member

Choose a reason for hiding this comment

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

We have dedicated pages for each storage backend now, I would rather consolidate everything about that backend there, including config examples. This page could be just about different deployment roles, how they are different, the ports, the APIs exposed.

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 put the snippets here just to provide some guidance. If you want me to just point to the storage pages, I can do that. Please clarify.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I would put them in the storage pages and link from here. I don't think we should focus so much on the backends in this top page.

jaegertracing/jaeger-query:{{< currentVersion >}}
```

!!! NEED TO FIX 2 sections BELOW
### Clock Skew Adjustment
Copy link
Member

Choose a reason for hiding this comment

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

need to move elsewhere, this is not a "deployment" section. Technically it belongs to query/UI role, so could move there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you want me to put an example UI/query role here for the deployment instead? I can add the snippets.

Copy link
Member

Choose a reason for hiding this comment

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

we need to mention the UI/query role on this page, but the actual configuration details can go to its own dedicated paged.

@jkowall
Copy link
Collaborator Author

jkowall commented Nov 1, 2024

Should be done with the updates and examples. This should fix : #750

Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro enabled auto-merge (squash) November 2, 2024 01:21
@yurishkuro yurishkuro merged commit 83ee5f0 into jaegertracing:main Nov 2, 2024
9 of 10 checks passed
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