Skip to content

Added publishing of plugin broker logs as runtime logs#11423

Merged
sleshchenko merged 7 commits intoeclipse-che:masterfrom
sleshchenko:pluginBrokerLogs
Oct 4, 2018
Merged

Added publishing of plugin broker logs as runtime logs#11423
sleshchenko merged 7 commits intoeclipse-che:masterfrom
sleshchenko:pluginBrokerLogs

Conversation

@sleshchenko
Copy link
Copy Markdown
Member

@sleshchenko sleshchenko commented Oct 1, 2018

What does this PR do?

The main purpose of this PR is adding publishing of plugin broker logs as runtime logs.
To achieve it there are several commits:

  • Introduced new event RuntimeLogEvent instead of MachineLogEvent. New event can be used to publish machine related log or machine independent log (like Plugin Broker). There is optional field machineName that indicates whether the event is machine related;
  • Add RuntimeId to plugin broker events instead of workspace id field. It is needed to publish PluginBrokerEvent as RuntimeLogEvent;
  • Make Che Server listen to broker log and republish it as runtime log;
  • Make Workspace Loader listen to runtime log instead of machine log;
  • Adapt GWT IDE to RuntimeLogEvent changes;

Dashboard is not adapted and it still uses deprecated machine/log JSON RPC method. A separated issue will be created for it.

Here is an example of how:

Workspace Loader displays Plugin Broker logs
screenshot_20181001_155728

Dashboard displays Plugin Broker logs
screenshot_20181003_154415

Minor improvements

  • to make plugin broker testing easier there is commit that sets Plugin Broker pull policy according to parameter;

What issues does this PR fix or reference?

#11069

Release Notes

N/A

Docs PR

N/A

@sleshchenko sleshchenko added the kind/enhancement A feature request - must adhere to the feature request template. label Oct 1, 2018
@sleshchenko sleshchenko self-assigned this Oct 1, 2018
@sleshchenko
Copy link
Copy Markdown
Member Author

@akurinnoy Please review workspace loader related changes 6b8166d
@azatsarynnyy Please review GWT IDE related changes 428ea8e

Copy link
Copy Markdown
Contributor

@akurinnoy akurinnoy left a comment

Choose a reason for hiding this comment

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

LGTM

@benoitf benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 1, 2018
@sleshchenko
Copy link
Copy Markdown
Member Author

Depends on eclipse-che/che-plugin-broker#12

Copy link
Copy Markdown
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

LGTM

@sleshchenko
Copy link
Copy Markdown
Member Author

@akurinnoy Please validate one more commit in this PR 97c170d

BrokerLogEvent withTime(String time);

/** Returns standard streams, if present otherwise, null will be returned. */
String getStream();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since broker doesn't work with RAW commands execution I would recommend to remove it from broker specific logs.

Copy link
Copy Markdown

@garagatyi garagatyi left a comment

Choose a reason for hiding this comment

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

Looks good!

@Override
public void onEvent(BrokerEvent event) {
if (!workspaceId.equals(event.getWorkspaceId())) {
if (!workspaceId.equals(event.getRuntimeId().getWorkspaceId())) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please, check that runtime is not NULL

case BROKER_STATUS_CHANGED_METHOD:
case BROKER_RESULT_METHOD:
workspaceId = ((BrokerStatusChangedEvent) params[0]).getWorkspaceId();
workspaceId = ((BrokerStatusChangedEvent) params[0]).getRuntimeId().getWorkspaceId();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please, check that runtimeId is not NULL

@sleshchenko
Copy link
Copy Markdown
Member Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 4, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11423
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@sleshchenko
Copy link
Copy Markdown
Member Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 4, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11423
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@sleshchenko sleshchenko merged commit 5b10f77 into eclipse-che:master Oct 4, 2018
@sleshchenko sleshchenko deleted the pluginBrokerLogs branch October 4, 2018 18:13
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 9, 2018
@benoitf benoitf added this to the 6.13.0 milestone Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement A feature request - must adhere to the feature request template.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants