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 docs page on Source details #2334

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

damemi
Copy link
Contributor

@damemi damemi commented Jan 29, 2025

This adds a new page on some of the under-the-hood details of how sources work and how they are meant to be used

@damemi damemi requested a review from BenElferink January 29, 2025 01:10
icon: 'circle-check'
---

This page explains how Source objects work in more detail, including best practices and recommended workflows.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this page should be part of the "introduction" page.
Intro should explain the "what is this and how to approach it". Right now for Sources, the intro only has supported workloads and the labels .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking was to add this as a deep dive page and keep the other pages like introduction/creating/deleting brief with only the key info. This goes a lot more into details that I think would be unnecessary or overwhelming in the intro. I did link to this page from the introduction though, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

It only seems like a lot because some of the info is duplicated:

For example the "Source Labels" which exist here and in introduction:

All Sources will automatically have 3 labels applied by a mutating webhook in the `instrumentor` component:

Or "Using multiple Sources" which exists here and in create:

You can have a namespace Source and a workload Source that both enable instrumentation for a workload. In this case, if one is deleted, the other will persist instrumentation for the workload.

I think if the duplicates are stripped down, or merged into their existing sections, then the rest of this file can be inserted in introduction.

It'll also be harder to maintain the docs in the future if we have multiple pages mentioning the same things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @BenElferink, I think you're right. I updated this to put the information in the Sources introduction page, ptal!

docs/pipeline/sources/working-with-sources.mdx Outdated Show resolved Hide resolved
Comment on lines 100 to 101
* When creating a Source grouping in the UI, Odigos will create a Workload Source for each Workload in the group.
* In v1.0.144+, when setting the `odigos-instrumentation` label on a workload or namespace, Odigos will automatically create a Source to migrate from the legacy label to Sources. Note that deleting the label will not remove the Source once it is automatically created.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think this will need revisiting once we implement the Source ownership? wonder if should keep this out until we have an implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should say exactly what happens right now, and then yes once we change how Sources work for ownership this should be updated

@damemi damemi force-pushed the source-details-docs branch 2 times, most recently from 7553f49 to 0fe233e Compare February 3, 2025 19:33
@damemi damemi force-pushed the source-details-docs branch from 0fe233e to a58667a Compare February 3, 2025 19:35
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