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

Closes #1543 - Older Agent versions are no longer compatible with the config server from the release 2.2. #1544

Merged
merged 21 commits into from
Nov 30, 2022

Conversation

danipaniii
Copy link
Contributor

@danipaniii danipaniii commented Nov 15, 2022

Fixing the incompatibility error with older agent versions and the config server from the release 2.2.0.


This change is Reviewable

@danipaniii
Copy link
Contributor Author

version-code

Personally I am wondering if the if-condition at line 122 is necessary?
Because by default log preloading, agent commands and the service states available are set to false and if they can't be fetched later in the try catch block at 140, they are not available.

@heiko-holz
Copy link
Contributor

components/inspectit-ocelot-configurationserver-ui/yarn.lock line 0 at r5 (raw file):
can you please delete your yarn.lock locally and build it fresh (via yarn install). I think the uploaded yarn.lock also contains npm stuff

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, 1 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @danipaniii)


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 140 at r5 (raw file):

      agentIdElement = <span style={{ color: 'gray' }}>({agentId})</span>;

      try {

I would surround this with if(serviceStatesAvailable), otherwise we are executing unnecessary code.


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 143 at r5 (raw file):

        serviceStates = JSON.parse(metaInformation.serviceStates);

        logAvailable = serviceStates.LogPreloader;

Actually, we need to retrieve the logAvailable and agentCommandsEnabled differently, to be downwards-compatible with versions that do not have the service states.

Thus, we need an if/else, checking whether serviceStatesAvailable. If it's not available due to an earlier version of the agent, I think we have to parse the config to see if logAvailable and agentCommandsEnabled


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 241 at r5 (raw file):

            logAvailable && agentCommandsEnabled
              ? 'Show Logs'
              : "<b>Logs not available!</b>\nMake sure to enable 'log-preloading' and 'agent-commands' in the config, and configure the URL for the agent commands.\nAvailable for agent versions 1.1.5 and above"

Can you add a similar message to the Download Support Archive tooltip :)?


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 291 at r5 (raw file):

    let agentCommandsEnabled = false;

    try {

Again, to be downwards compatible, we need to check if serviceStatesAvailable, and if not, retrieve the agentCommandsEnabled differently.

@heiko-holz
Copy link
Contributor

Hi @danipaniii , I think we still need line 122 it to see if serviceStatesAvailable, to use it further (see my other comments).

If we don't check for serviceStatesAvailable (see other comments), we are executing unnecessary try/catch blocks.
Also, we need to retrieve logAvailable´ and agentCommandsEnabled` in a downward-compatible way (see my other comments)

Copy link
Member

@MariusBrill MariusBrill left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @danipaniii and @heiko-holz)


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 140 at r5 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

I would surround this with if(serviceStatesAvailable), otherwise we are executing unnecessary code.

done.


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 143 at r5 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

Actually, we need to retrieve the logAvailable and agentCommandsEnabled differently, to be downwards-compatible with versions that do not have the service states.

Thus, we need an if/else, checking whether serviceStatesAvailable. If it's not available due to an earlier version of the agent, I think we have to parse the config to see if logAvailable and agentCommandsEnabled

As we discussed, we won't implement this feature as it would too much load on the client


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 241 at r5 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

Can you add a similar message to the Download Support Archive tooltip :)?

done.


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 291 at r5 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

Again, to be downwards compatible, we need to check if serviceStatesAvailable, and if not, retrieve the agentCommandsEnabled differently.

done.

Copy link
Member

@Dimi-Ma Dimi-Ma left a comment

Choose a reason for hiding this comment

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

Dismissed @heiko-holz from a discussion.
Reviewable status: 1 of 4 files reviewed, 4 unresolved discussions (waiting on @heiko-holz and @MariusBrill)


components/inspectit-ocelot-configurationserver-ui/yarn.lock line at r5 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

can you please delete your yarn.lock locally and build it fresh (via yarn install). I think the uploaded yarn.lock also contains npm stuff

done

Copy link
Member

@MariusBrill MariusBrill left a comment

Choose a reason for hiding this comment

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

Dismissed @heiko-holz from 2 discussions.
Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @Dimi-Ma and @heiko-holz)

Dimi-Ma
Dimi-Ma previously approved these changes Nov 17, 2022
Copy link
Member

@Dimi-Ma Dimi-Ma left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @heiko-holz and @MariusBrill)

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 3 files at r6, 1 of 2 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @danipaniii)


components/inspectit-ocelot-configurationserver-ui/src/components/views/dialogs/DownloadDialogue.js line 29 at r7 (raw file):

        return `${dialogueSettings.header} could not be loaded due to an unexpected error.\n
                Ensure that both agent-commands and log-preloading are enabled and the agent-commands URL is correct in your agent
                configuration.\nThis feature is only available for agent versions 1.15.0 and higher`;

I added the note for the required version number to the error message in the DownloadDialogue


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 111 at r7 (raw file):

    const agentVersionTokens = agentVersion.split('.');
    let logAvailable = false;
    let agentCommandsEnabled = false;

agentCommandsEnabled will never be set to true if the serviceStatesAvailable is not true.
I think we need to include a version check to the agent commands as well.


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 125 at r7 (raw file):

      logAvailable = agentVersionNumber > 11500;
      // support archive is available at version 2.20+
      supportArchiveAvailable = agentVersionNumber >= 22000;

I added supportArchiveAvailable


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 127 at r7 (raw file):

      supportArchiveAvailable = agentVersionNumber >= 22000;
      // service states are available at version 2.20+
      serviceStatesAvailable = agentVersionNumber >= 22000;

I fixed it to 22000


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 112 at r8 (raw file):

    const agentVersionTokens = agentVersion.split('.');
    let logAvailable = false;
    let agentCommandsEnabled = false;

the agentCommandsEnabled will never be true unless we have serviceStatesAvailable.

I think we need to also do a version check for agent commands, or assume agent commands are enabled.


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 132 at r8 (raw file):

    }

    if (serviceStatesAvailable) {

I added the code snippet to resolve the serviceStates here, to avoid duplicate code

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r11, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @danipaniii)


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 112 at r11 (raw file):

    const agentVersionTokens = agentVersion.split('.');
    let logAvailable = false;
    let agentCommandsEnabled = true;

We now initialize the agentCommandsEnabled to true (to be downwards compatible)

Copy link
Contributor Author

@danipaniii danipaniii left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 5 files reviewed, 3 unresolved discussions (waiting on @Dimi-Ma, @heiko-holz, and @MariusBrill)


components/inspectit-ocelot-configurationserver-ui/src/components/views/dialogs/DownloadDialogue.js line 29 at r7 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

I added the note for the required version number to the error message in the DownloadDialogue

Nice!


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 127 at r7 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

I fixed it to 22000

I am still seeing the 20200
image.png
I fixed it.


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 360 at r11 (raw file):

                agentCommandsEnabled && supportArchiveAvailable
                  ? 'Download Support Archive'
                  : "<b>Support archive not available!</b>\nMake sure to enable 'agent-commands' in the config and configure the URL for the agent commands. \n This feature is only available for agent versions 1.15.0 and above."

Changed the version in the tooltip to 2.2.0

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r12, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @danipaniii)


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 127 at r7 (raw file):

Previously, danipaniii wrote…

I am still seeing the 20200
image.png
I fixed it.

Actually, I was "unfixing" it 👼

The actual agent version is 20200 (as the minor version is 2, in 22000 the minor version is 20.

Let's discuss this in today's Sync meeting.


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 360 at r11 (raw file):

Previously, danipaniii wrote…

Changed the version in the tooltip to 2.2.0

thanks!

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r13, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @danipaniii)

@heiko-holz heiko-holz merged commit f5d23ec into inspectIT:master Nov 30, 2022
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.

None yet

4 participants