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

UI - New Operations tab #1600

Merged
merged 8 commits into from
Mar 30, 2023
Merged

Conversation

thfries
Copy link
Contributor

@thfries thfries commented Mar 13, 2023

please find this PR resolves #1590

  • added new tab
  • changed way to control right auth user
  • extended api to allow devops path
  • utils to add table row now returns row
  • crud editor returns explicit cancel action
  • crud editor now with option without delete or create

image

Please let me know if this is the right direction.

Pending topics:

  • setting log level for all services is currently not possible
  • I m thinking if we should allow to hide unused tabs by configuration (e.g. hide connections and operations for sandbox)
  • Need to add validation to avoid invalid json to be used on update

Best regards, Thomas

* added new tab
* changed way to control right auth user
* extended api to allow devops path
* utils to add table row now returns row
* crud editor returns explicit cancel action
* crud editor now with option without delete or create

Signed-off-by: thfries <thomas.fries0@gmail.com>
@thfries thfries changed the title UI - New Operations tab #1590 UI - New Operations tab Mar 13, 2023
@thjaeckle
Copy link
Member

Hi @thfries
Thanks for the fast first draft 👍

I thought that the different available log levels for each logger could be rendered with a bootstrap button group, e.g. also having different colors - DEBUG maybe light gray, INFO green, WARN yellow and ERROR red.
And the currently active log level could be highlighted in order to see at once glance what is currently configured.

That way, the buttons could show the currently configured log level and could also be used to directly change it.
Table + Editor for this use case seem a little bit too complicated, in my opinion.

Signed-off-by: thfries <thomas.fries0@gmail.com>
@thfries
Copy link
Contributor Author

thfries commented Mar 18, 2023

Hey @thjaeckle,
thanks for your proposal. Usability is indeed far better and the code is even simpler as before. This is how it looks like now:
image

I'm not sure about the refresh button: It should not scroll away or may be it is not needed at all?

Best regards, Thomas

@thfries
Copy link
Contributor Author

thfries commented Mar 18, 2023

added another commit to keep the refresh button visible...

@thjaeckle
Copy link
Member

@thfries yes, that looks really great. 👍
Will take a look at the code soon.

I think the api supports changing the log level of e.g. all connectivity instances at once and even of specific instances separately.
In my experience, only changing the levels of all instances at once is required.

@thjaeckle thjaeckle added this to the 3.2.1 milestone Mar 20, 2023
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

Hi @thfries
I just reviewed - looks good 👍
Simple and adds a lot benefit for operating Ditto.

Just 2 newly created files have the wrong copyright year, I mentioned that inline.

Apart from that this would be good for me to merge.

ui/modules/operations/operations.html Outdated Show resolved Hide resolved
ui/modules/operations/operations.js Outdated Show resolved Hide resolved
@thjaeckle
Copy link
Member

@thfries I totally forgot .. There is another thing to do which would this make even more useful:

There should be for each service (e.g. "things", "gateway") a textfield in which you can enter a new arbitrary logger and also select a log level.
The API supports to add new loggers in the fly - that is e.g. very useful if you just need to have DEBUG logs for a single class - then you just add a logger on-the fly with DEBUG - and e.g. deactivate again after you have collected some DEBUG logs.

And they can even be removed again by setting the log level to "OFF" - which we could also think about.

Signed-off-by: thfries <thomas.fries0@gmail.com>
@thfries
Copy link
Contributor Author

thfries commented Mar 23, 2023

Hi @thjaeckle ,
thanks for your feedback and your thoughts.
Review comments are fixed.
Regarding your other comments:

  • I found no API call to set all loggers of one service at once. But in general we can add a "select all" button on top of all columns to do that. If we need several API calls this would still be possible
  • There is an API call to set the level for one logger for all services => therefor we would need a checkbox "apply for all services" on each row?
  • adding and deleting loggers sounds interesting, too. If this is a relevant use case, we could add it.

The first one sounds very valuable to still add.
...and I still would like to add a feature to hide unused tabs by the environment definition 😉

If you like, I can add some more improvements to this PR.
I will remove draft state of the PR, so you can decide to merge... as you like

Thank you, @thjaeckle

@thfries thfries marked this pull request as ready for review March 23, 2023 18:29
@thfries thfries requested a review from thjaeckle March 23, 2023 18:30
@thfries thfries marked this pull request as draft March 25, 2023 11:01
- fixed auth header mixing from wrong environment
- operations tab not getting right auth header

Signed-off-by: thfries <thomas.fries0@gmail.com>
@thfries
Copy link
Contributor Author

thfries commented Mar 26, 2023

Hi @thjaeckle,
added the toggles for the main menu and found a bug that the auth header was keeping active after switching an environment.

@thfries thfries marked this pull request as ready for review March 26, 2023 12:47
@thjaeckle
Copy link
Member

  • I found no API call to set all loggers of one service at once. But in general we can add a "select all" button on top of all columns to do that. If we need several API calls this would still be possible

Your proposal already uses this API to switch all loggers of one service at once - so that works as intended 👍

  • There is an API call to set the level for one logger for all services => therefor we would need a checkbox "apply for all services" on each row?

I don't think that this would be needed.

  • adding and deleting loggers sounds interesting, too. If this is a relevant use case, we could add it.

Yes, this is really valuable, as you can e.g. define for a specific class or package a new logger config without that it needs to be "visible" before in the logging overview.
I would prefer to already have it included in this PR - I could also try to contribute that if I find time ..

Please share your thoughts on that :)

@thjaeckle
Copy link
Member

@thfries somehow the "Bearer" token for the main user gets lost now when reloading the page via the browser.
Do you also have that behavior?

I managed to add a input field for adding new loggers - and that integrates quite well into your approach.
Will push shortly so you can have a look.
If it is not ok, we can revert the commit again ..

thjaeckle and others added 2 commits March 29, 2023 21:35
@thfries
Copy link
Contributor Author

thfries commented Mar 30, 2023

Hi @thjaeckle ,

nice, thank you!
Yes I also observed issues with the bearer but I was not yet able to reproduce it.
Your addition looks nice. I made one comment and I already thought about to split into smaller local functions to improve readability a bit (I think this is best practice). Let me give it a try.

- remove spellcheck from input
- split loggerView into smaller functions

Signed-off-by: thfries <thomas.fries0@gmail.com>
@thfries
Copy link
Contributor Author

thfries commented Mar 30, 2023

Done. I like your approach. It keeps the needed parameters just where they are needed. Works also for the other buttons. Strange that it didn't work with the event target...

Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

Lgtm

@thjaeckle thjaeckle merged commit d33da83 into eclipse-ditto:master Mar 30, 2023
thjaeckle added a commit that referenced this pull request Mar 31, 2023
* UI - New Operations tab
* added new tab
* changed way to control right auth user
* extended api to allow devops path
* utils to add table row now returns row
* crud editor returns explicit cancel action
* crud editor now with option without delete or create

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - Operations Tab: new logger view

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - operations tab - refresh not scrolling

* UI - change header info on new files

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - Operations tab - make tabs optional
- fixed auth header mixing from wrong environment
- operations tab not getting right auth header

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - added option to define new logger for each service in Operations / Logging functionality

Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>

* UI - operations tab
- remove spellcheck from input
- split loggerView into smaller functions

Signed-off-by: thfries <thomas.fries0@gmail.com>

---------

Signed-off-by: thfries <thomas.fries0@gmail.com>
Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
Co-authored-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
@thfries thfries deleted the ui-operations-tab branch March 31, 2023 05:18
thfries added a commit to thfries/ditto that referenced this pull request Mar 31, 2023
* UI - New Operations tab
* added new tab
* changed way to control right auth user
* extended api to allow devops path
* utils to add table row now returns row
* crud editor returns explicit cancel action
* crud editor now with option without delete or create

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - Operations Tab: new logger view

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - operations tab - refresh not scrolling

* UI - change header info on new files

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - Operations tab - make tabs optional
- fixed auth header mixing from wrong environment
- operations tab not getting right auth header

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - added option to define new logger for each service in Operations / Logging functionality

Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>

* UI - operations tab
- remove spellcheck from input
- split loggerView into smaller functions

Signed-off-by: thfries <thomas.fries0@gmail.com>

---------

Signed-off-by: thfries <thomas.fries0@gmail.com>
Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
Co-authored-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
alstanchev pushed a commit to bosch-io/ditto that referenced this pull request Jul 17, 2023
* UI - New Operations tab
* added new tab
* changed way to control right auth user
* extended api to allow devops path
* utils to add table row now returns row
* crud editor returns explicit cancel action
* crud editor now with option without delete or create

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - Operations Tab: new logger view

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - operations tab - refresh not scrolling

* UI - change header info on new files

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - Operations tab - make tabs optional
- fixed auth header mixing from wrong environment
- operations tab not getting right auth header

Signed-off-by: thfries <thomas.fries0@gmail.com>

* UI - added option to define new logger for each service in Operations / Logging functionality

Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>

* UI - operations tab
- remove spellcheck from input
- split loggerView into smaller functions

Signed-off-by: thfries <thomas.fries0@gmail.com>

---------

Signed-off-by: thfries <thomas.fries0@gmail.com>
Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
Co-authored-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
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.

UI: Enhance Ditto-UI to configure log levels of Ditto
2 participants