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

Alarm log table refactor #3170

Merged

Conversation

jeonghanlee
Copy link
Contributor

@jeonghanlee jeonghanlee commented Oct 18, 2024

Hi Kay @kasemir and Kunal @shroffk

We would like to suggest a small workaround solution for the alarm log table layout and its headers.

We have been struggling to figure out what each of the alarm log table headers means. Now we have intensively studied their physical meanings. Then we saw some problems with the alarm log table. Here is our "workaround" solution to minimize any impact on the existing table definitions.

We don't change their original definitions, but introduce a method to define all header names in a site-specific way. We have also added the ability to configure the visibility of the Alarm Log table in a similar and limited way.

Our intention is to implement their configuration dynamically, but due to the mixed configurations from everywhere, we decide to "replace" the site-specific configuration during the build process. This keeps our work to a minimum and keeps all other sites the same as before.

  • Fixed the mismatch time definition in the search bar to use its message time instead of time. We selected message time because the log table also contains "ACK" events. (Currently method does not provide the clear way to see the exact alarm event if user wants to focus the time However, the previous our PR, the time delta should be in the acceptable range in most case. Thus, we use the message time (It is quite confusing, I prefer Alarm Log Time).
  • Added the alarm value option to the alarm log table since we already record the value changes within the alarm log service. (Currently method has one issue, which we will do another PR later)
  • See attached file with message time (default) and value (after execution, I enabled the option)

1

The configurable options are defined in messages.properties. We kept it as the same as before.

https://github.com/jeonghanlee/phoebus/blob/2a92aa6f0e07b1a3f265c285fc73cc0a9ef79ea6/app/alarm/logging-ui/src/main/resources/org/phoebus/applications/alarm/logging/ui/messages.properties

Technically, based on this pull request, we would like to redefine our Alarm Log Table according to the following file

https://github.com/jeonghanlee/phoebus-env/blob/master/site-template/alarm-logger_logtable-messages.properties

And its screen is here also.

2

With this method, we have zero impact on the current Phoebus Alarm Log Table, and give ourselves quite a bit of flexibility to optimize our environment without much discussion.

Some day later, I want to discuss all definitions of Alarm Table and Alarm Log Tables with you. However, we don't have enough resources to make it look perfect.

Thanks,

@jeonghanlee, Soo Ryu, and @Sangil-Lee at ALS-U Controls, LBNL.

@kasemir
Copy link
Collaborator

kasemir commented Oct 21, 2024

If you want a site specific view of the alarm log, Kibana allows you to do anything, and that's in fact how I'm looking at the alarm log.
So I'll leave decisions about the alarm log table to Kunal.

@jeonghanlee
Copy link
Contributor Author

@kasemir Not actually site-specific view, but try to challenge to understand what the title means without any impact on other sites.

Please check the comments on our messages.properties file. Even if the Site-specific view, we don't want to introduce new services if the Phoebus has the option.

Current table titles do not have the consistent definitions between Alarm Table and Alarm Log. And we gave an option to change them before I will bring the intense discussion on them.

@shroffk
Copy link
Member

shroffk commented Oct 21, 2024

I think these changes are fine with me for now.

I would like to understand the broader requirements you have since redesigning the alarm log table has been on the codeathon project list for the last 2 codeathon and we have simply not gotten to it... I would like to take up this project soon if possible ( and your input would be very useful )

The main usecase I have observed is to go from alarm tree/table --> context menu Open alarm log history --> alarm log table.
This workflow, which uses the integrated Phoebus environment, is very useful to someone trying to understand a particular alarm. Kibana is very good for building dashboard, gathering statistics, and figuring out how to improve your alarm configuration. Both these clients are very useful in the alarm services eco system.

@shroffk shroffk merged commit 72b8cc9 into ControlSystemStudio:master Oct 21, 2024
1 check passed
@shroffk
Copy link
Member

shroffk commented Oct 21, 2024

About the titles,

The alarm log view uses all the raw message fields... this might obviously not be great. I understand you are fixing this using the messages but I am also open to the Idea of revisiting and improving the column names.
Ensuring the column names match across alarm table and alarm log table seems like an obvious first step.

@jeonghanlee
Copy link
Contributor Author

@shroffk I will call a short meeting with you, or maybe @kasemir if he wants to join. We don't have many requirements except for some work since we don't have much time to define the production level of the alarm services for me.

  • We would like to add latch status to the commands we are working on. Without latch information, I don't think the service has all the (critical) alarm information.

  • The value should be changed in the same severity state, such as the following case LOLO : Major -> HIHI : Major. Even if the same Major severity, the value has changed significantly, so for troubleshooting purposes we need the exact alarm value. We can do this in a similar way with the Archiver Appliance. However, the time between two services is not exactly the same.

  • The alarm log table with the different states looks very ugly and I would like to redesign it, but this is not a real requirement at the moment. Nice to have, but not a must.

So, this time, we are working on two subjects only, after them, I think, the alarm services will be our production level product for the operation. We will see how much we will go.

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