Skip to content

Conversation

@PeterYurkovich
Copy link
Contributor

Background

Issue: https://issues.redhat.com/browse/OU-269

Any number of TempoStack instances may exist within a cluster, including those in different namespaces or with the same name. This PR looks to add a dropdown to allow the user to select which TempoStack instance they would like to have apply to the rest of the tracing UI components.

Implementation

Once selected the namespace and name are added to the URL as query parameters for reproducibility as is standard within the OpenShift Observability ecosystem. A new useTempoStack hook has been provided to retrieve the various tempo instances.

Image

Screenshot 2024-03-01 at 3 58 49 PM

Add Select example from patternfly to project then pass in LokiStack list. Add some todo's to direct progress
Since name aren't unique across other namespaces, the namespace is needed as well. We are encoding it as another query parameter, but not showing the data underneath unless the combination exists
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 1, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 1, 2024

@PeterYurkovich: This pull request references OU-269 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

Background

Issue: https://issues.redhat.com/browse/OU-269

Any number of TempoStack instances may exist within a cluster, including those in different namespaces or with the same name. This PR looks to add a dropdown to allow the user to select which TempoStack instance they would like to have apply to the rest of the tracing UI components.

Implementation

Once selected the namespace and name are added to the URL as query parameters for reproducibility as is standard within the OpenShift Observability ecosystem. A new useTempoStack hook has been provided to retrieve the various tempo instances.

Image

Screenshot 2024-03-01 at 3 58 49 PM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested review from jgbernalp and kyoto March 1, 2024 21:04
@PeterYurkovich
Copy link
Contributor Author

/jira refresh

@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 1, 2024

@PeterYurkovich: This pull request references OU-269 which is a valid jira issue.

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Comment on lines 27 to 36
const tempoList: Array<K8sResourceCommon> = [];
if (Array.isArray(list)) {
list.forEach((tempoStack) => {
tempoList.push(tempoStack);
});
} else {
list.items.forEach((tempoStack) => {
tempoList.push(tempoStack);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const tempoList: Array<K8sResourceCommon> = [];
if (Array.isArray(list)) {
list.forEach((tempoStack) => {
tempoList.push(tempoStack);
});
} else {
list.items.forEach((tempoStack) => {
tempoList.push(tempoStack);
});
}
let tempoList: Array<K8sResourceCommon> = [];
if (Array.isArray(list)) {
tempoList = list;
} else {
tempoList = list.items;
}

);

if (!tempoStackList) {
return <div>Loading...</div>;
Copy link
Contributor

Choose a reason for hiding this comment

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

This text might need translation

) => void;
};

export const TempoDropdown = (props: TempoDropdownProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const TempoDropdown = (props: TempoDropdownProps) => {
export const TempoStackDropdown = (props: TempoDropdownProps) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed function and file to TempoStackDropdown

if (value && value !== 'no results') {
setSelected(value as string);
}
const [namespace, tempoStackName] = String(value).split(' / ');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a better way to get this data from an object rather than splitting a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used the SelectOptionObject interface from PatternFly to use an object directly as the value in the Dropdown List

isDisabled={option.disabled}
key={index}
value={option.value}
{...(option.description && { description: option.description })}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be the same?

Suggested change
{...(option.description && { description: option.description })}
description={option.description ?? undefined}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Description wasn't needed after changes

@jgbernalp jgbernalp merged commit 94d5e8c into openshift:main Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants