-
Notifications
You must be signed in to change notification settings - Fork 58
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
Multi-tenancy support for trace analytics #1404
base: main
Are you sure you want to change the base?
Multi-tenancy support for trace analytics #1404
Conversation
Signed-off-by: Julian Quispel <[email protected]>
Signed-off-by: Julian Quispel <[email protected]>
…oards-observability into feature/multitenancy
Signed-off-by: Julian Quispel <[email protected]>
Signed-off-by: Julian Quispel <[email protected]>
…oards-observability into feature/multitenancy
Signed-off-by: Julian Quispel <[email protected]>
…oards-observability into feature/multitenancy
Is there perhaps any update on this? I would like this functionality for my own setup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding this, overall looks good. added some minor comments
public/components/application_analytics/components/config_components/service_config.tsx
Outdated
Show resolved
Hide resolved
public/components/application_analytics/components/flyout_components/service_detail_flyout.tsx
Outdated
Show resolved
Hide resolved
public/components/trace_analytics/components/dashboard/dashboard_content.tsx
Outdated
Show resolved
Hide resolved
public/components/trace_analytics/components/traces/trace_view.tsx
Outdated
Show resolved
Hide resolved
@joshuali925 thanks for your reply. I addressed your comments 😄 |
Signed-off-by: Julian Quispel <[email protected]>
74e81e0
to
b6b8117
Compare
Signed-off-by: Joshua Li <[email protected]>
@JulianQuispel thanks for the update! i fixed some errors i ran into when pulling it down |
Signed-off-by: Shenoy Pratik <[email protected]>
public/components/trace_analytics/components/services/services_content.tsx
Show resolved
Hide resolved
body: schema.object({ | ||
tenant: schema.maybe(schema.string()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this be a problem to add a hard enforced schema in as compared to any
before? This is because with this change we aren't other fields passed in body apart from tenant today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i didn't understand the question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the body contain other fields and should they be added in the schema?
public/components/trace_analytics/requests/dashboard_request_handler.ts
Outdated
Show resolved
Hide resolved
public/components/trace_analytics/requests/dashboard_request_handler.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Joshua Li <[email protected]> Signed-off-by: Julian Quispel <[email protected]>
…oards-observability into feature/multitenancy
@joshuali925 @ps48 thanks for the replies. I addressed most of your suggestions/remarks. |
Since the merge conflicts are solved, what is the status on this PR and could it be approved + merged? |
Is there possibly an update on this? |
@JulianQuispel, @JoeriRoijenga, @RemcoStoelwinder we are releasing an experimental feature in 2.17 to have custom source for trace analytics. This feature custom index names with supports for wildcards, cross cluster searches. The experimental implementation stores the custom index names in advanced settings which close to what this PR brings in. Related PR can be found here: #2112 We also, have a roadmap ahead where we added pointers on the direction we want to take with the plugin in the coming releases. It would be great if you can take a look/comment and share your views: |
Description
Enable multi-tenancy in the dashboards observability plugin. This way tenants only have access to their own trace data
Issues Resolved
Closes #1377
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.