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

[2.0] Dashboards Plugin Errors #1206

Closed
boktorbb opened this issue Feb 3, 2022 · 10 comments
Closed

[2.0] Dashboards Plugin Errors #1206

boktorbb opened this issue Feb 3, 2022 · 10 comments
Assignees
Labels
migration Any plans, changes, or enhancements needed for migration nodejs 🍭 issues related to nodejs client plugins Plugin related work release >upgrade

Comments

@boktorbb
Copy link
Contributor

boktorbb commented Feb 3, 2022

This is the place to ask questions and get insights into errors you run into as you work on preparing your Plugin for 2.0. The main branch currently has the Node v14 upgrade as well as several dependency upgrades for CVE fixes. The main branch is stable and working for Core Dashboards but may not be stable for downstream plugins.

NOTE: The errors and issues posted here are not a list of things the core dashboards team will take on fix. This is a platform to engage the Core team on issues that you may need more guidance on or questions that you may have after your initial debugging work.

@boktorbb boktorbb added plugins Plugin related work migration Any plans, changes, or enhancements needed for migration v2.0.0 >upgrade nodejs 🍭 issues related to nodejs client labels Feb 3, 2022
@kavilla
Copy link
Member

kavilla commented Feb 3, 2022

Note there were some NPM packages there we upgraded to mitigate CVEs, there might be some conflict of package versions in your plugin for example: #1146

Also make sure to update your package.json and opensearch_dashboards.json to be 2.0.0 or 2.0.0.0 in the appropriate locations for example:

For runtime, I would make sure if you have an OpenSearch plugin has a 2.0.0.0 plugin as well.

Well upgrading to 2.0.0.0, you can then add it to the build pipeline here:
https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.0.0/opensearch-dashboards-2.0.0.yml

@ananzh
Copy link
Member

ananzh commented Mar 14, 2022

For plugins hookup with the lastest osd with opensearch-js client, please refer to this issue .

@amitgalitz
Copy link
Member

I followed the steps given in this issue for plugin hookup (Anomaly Detection). The first issue I experienced was related to package mismatch that was easy to fix. The other issues I am experiencing and not sure how to solve yet are that a lot of the AD requests from the frontend are getting a 400 bad request response. Specifically a lot of them such as http://localhost:5601/api/anomaly_detectors/detectors/test-name/_match or http://localhost:5601/api/anomaly_detectors/_indices?index= are getting this message back: "[request body]: expected value of type [any] but got [undefined]". When I directly hit the backend with the equivalent backend API there are no problems.

@ashwin-pc ashwin-pc assigned ananzh and unassigned boktorbb Mar 28, 2022
@ananzh
Copy link
Member

ananzh commented Mar 31, 2022

Issues in AD hookup:

[1] 400 bad request

{"statusCode":400,"error":"Bad Request","message":"[request body]: expected value of type [any] but got [undefined]"}

Issue link here

Issue is due to this line here

After typescript upgrade, Dashboards use a more restrict type check. For the get API from AD, we could see body is null.
Screen Shot 2022-03-30 at 9 28 11 PM

Body null is transformed to undefined in the type check in the dashboards. Then, type undefined is no longer acceptable from type any by restricter type check.

There are two ways to change:

  1. Seems request body is always null in get API. Could delete L64.
  2. Change to body: schema.nullable(schema.any()),

Both would solve the issue.

[2] AnnotationDomainTypes no longer exists in @elastic/charts
AnnotationDomainTypes does not exists due to @elastic/charts upgrade. This is the breaking change. AnnotationDomainType is the one used in @elastic/charts. Detector page can be rendered out.

Screen Shot 2022-03-30 at 11 13 09 PM'

Issue link here

Two ways to change:

  1. a quick fix, change AnnotationDomainTypes to
 AnnotationDomainType as AnnotationDomainTypes
  1. use AnnotationDomainType and modify all the places use AnnotationDomainTypes

[3] Other issues
For this file /public/pages/Dashboard/Components/AnomaliesLiveChart.tsx
I see there are two complains:

  1. showLegendDisplayValue={false}
Type '{ showLegend: boolean; legendPosition: "right"; showLegendExtra: false; showLegendDisplayValue: boolean; xDomain: { min: number; max: number; }; }' is not assignable to type 'IntrinsicAttributes & Partial<Omit<SettingsSpec, "id" | "chartType" | "specType">> & { children?: ReactNode; }'.
  Property 'showLegendDisplayValue' does not exist on type 'IntrinsicAttributes & Partial<Omit<SettingsSpec, "id" | "chartType" | "specType">> & { children?: ReactNode; }'.ts(2322)

Could simply remove this line since it is not used.

  1. LineAnnotation has no id , here
Property 'id' is missing in type '{ domainType: "xDomain"; dataValues: LineAnnotationDatum[]; style: { line: { strokeWidth: number; stroke: string; dash: number[]; opacity: number; }; }; marker: string; }' but required in type 'SpecRequiredProps'.ts(2741)
[index.d.ts(5, 5): ]()'id' is declared here.

Could add id here

                   <LineAnnotation
+                    id={'lineAnnotation'}

This issue is not a blocker.

After these changes, AD plugin can run on current Dashboards 2.0. Please help me to verify.

I could now create a detector:
Screen Shot 2022-03-30 at 9 31 08 PM

Then detector is rendered out:
Screen Shot 2022-03-30 at 11 14 13 PM

@ananzh
Copy link
Member

ananzh commented Mar 31, 2022

See unit tests all fail for AD. Continue deep dive here. Some issues:

[1] test-env is missing

"The error below may be caused by using the wrong test environment, see https://jestjs.io/docs/configuration#testenvironment-string. Consider using the "jsdom" test environment."

I think probably related to we taking out the test environment in src/dev/jest/config.js and only having it in the npm package. So the solution would be adding back in jest.config.js:

testEnvironment: 'jest-environment-jsdom',

[2] wait does not exist in @testing-library/react
Solution would be import waitFor from @testing-library/react, then switch from wait to waitFor. Here, waitFor requires a callback fun. If original wait() has non argument, you could pass an empty callback. For example,

await waitFor(()=>{});

[3] see some errors regarding EuiPanel

 FAIL  public/pages/Overview/containers/__tests__/AnomalyDetectionOverview.test.tsx (7.896 s)
  ● <AnomalyDetectionOverview /> spec › Some detectors created › renders component with sample detector

    TestingLibraryElementError: Unable to find an element with the text: INSTALLED. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

    Ignored nodes: comments, <script />, <style />
    <body>
      <div>
        <header
          class="euiPageHeader euiPageHeader--responsive euiPageHeader--center"
        >
          <div
            class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--justifyContentSpaceBetween euiFlexGroup--directionRow euiFlexGroup--responsive"
          >
            <div
              class="euiFlexItem euiFlexItem--flexGrowZero"
            >
              <h1
                class="euiTitle euiTitle--large"
                data-test-subj="overviewTitle"
              >
                Anomaly detection
              </h1>
            </div>
            <div
              class="euiFlexItem euiFlexItem--flexGrowZero"
            >
              <a
                class="euiButton euiButton--primary euiButton--fill"
                data-test-subj="add_detector"
                href="anomaly-detection-dashboards#/create-detector-steps"
                rel="noreferrer"
              >
                <span
                  class="euiButtonContent euiButton__content"
                >
                  <span
                    class="euiButton__text"
                  >
                    Create detector
                  </span>
                </span>
              </a>
            </div>
          </div>
        </header>
        <div
          class="euiText euiText--medium"
        >
          The anomaly detection plugin automatically detects anomalies in your data in near real-time using the Random Cut Forest (RCF) algorithm.
           
          <a
            class="euiLink euiLink--primary"
            href="https://opensearch.org/docs/monitoring-plugins/ad"
            rel="noopener noreferrer"
            target="_blank"
          >
            Learn more 
            <svg
              aria-hidden="true"
              class="euiIcon euiIcon--small"
              focusable="false"
              height="16"
              role="img"
              viewBox="0 0 16 16"
              width="16"
              xmlns="http://www.w3.org/2000/svg"
            >
              <path
                d="M13 8.5a.5.5 0 111 0V12a2 2 0 01-2 2H4a2 2 0 01-2-2V4a2 2 0 012-2h3.5a.5.5 0 010 1H4a1 1 0 00-1 1v8a1 1 0 001 1h8a1 1 0 001-1V8.5zm-5.12.339a.5.5 0 11-.706-.707L13.305 2H10.5a.5.5 0 110-1H14a1 1 0 011 1v3.5a.5.5 0 11-1 0V2.72L7.88 8.838z"
              />
            </svg>
            <svg
              aria-label="External link"
              class="euiIcon euiIcon--small euiLink__externalIcon"
              focusable="false"
              height="16"
              role="img"
              viewBox="0 0 16 16"
              width="16"
              xmlns="http://www.w3.org/2000/svg"
            >
              <path
                d="M13 8.5a.5.5 0 111 0V12a2 2 0 01-2 2H4a2 2 0 01-2-2V4a2 2 0 012-2h3.5a.5.5 0 010 1H4a1 1 0 00-1 1v8a1 1 0 001 1h8a1 1 0 001-1V8.5zm-5.12.339a.5.5 0 11-.706-.707L13.305 2H10.5a.5.5 0 110-1H14a1 1 0 011 1v3.5a.5.5 0 11-1 0V2.72L7.88 8.838z"
              />
            </svg>
            <span
              class="euiScreenReaderOnly"
            >
              (opens in a new tab or window)
            </span>
          </a>
        </div>
        <div
          class="euiSpacer euiSpacer--xl"
        />
        <div
          class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--hasShadow"
          style="padding: 20px;"
        >
          <div
            class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--alignItemsCenter euiFlexGroup--justifyContentSpaceBetween euiFlexGroup--directionRow euiFlexGroup--responsive"
            style="padding: 0px;"
          >
            <div
              class="euiFlexItem"
            >
              <h3
                class="euiTitle euiTitle--small"
                data-test-subj="contentPanelTitle"
              >
                How it works
              </h3>
              <div
                class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionRow euiFlexGroup--responsive"
              >
                <div
                  class="euiFlexItem content-panel-subTitle"
                  style="line-height: normal; max-width: 75%;"
                />
              </div>
            </div>
            <div
              class="euiFlexItem euiFlexItem--flexGrowZero"
            >
              <div
                class="euiFlexGroup euiFlexGroup--gutterMedium euiFlexGroup--alignItemsCenter euiFlexGroup--justifyContentSpaceBetween euiFlexGroup--directionRow euiFlexGroup--responsive"
              >
                <div
                  class="euiFlexItem"
                />
              </div>
            </div>
          </div>
          <div>
            <hr
              class="euiHorizontalRule euiHorizontalRule--full euiHorizontalRule--marginSmall"
            />
            <div
              style="padding: 10px 0px;"
            >
              <div
                class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionRow euiFlexGroup--responsive"
              >
                <div
                  class="euiFlexItem"
                >
                  <div
                    class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionColumn euiFlexGroup--responsive"
                  >
                    <div
                      class="euiFlexItem euiFlexItem--flexGrowZero"
                      style="margin-bottom: 5px;"
                    >
                      <div
                        class=...

       99 |       expect(getAllByText('Detector created')).toHaveLength(1);
      100 |       expect(getAllByText('View detector and sample data')).toHaveLength(1);
    > 101 |       expect(getAllByText('INSTALLED')).toHaveLength(1);
          |              ^
      102 |     });
      103 |     test('renders component with non-sample detector', async () => {
      104 |       httpClientMock.get = jest.fn().mockResolvedValue({

      at Object.getElementError (../../node_modules/@testing-library/dom/dist/config.js:38:19)
      at ../../node_modules/@testing-library/dom/dist/query-helpers.js:90:38
      at getAllByText (../../node_modules/@testing-library/dom/dist/query-helpers.js:130:20)
      at _callee2$ (public/pages/Overview/containers/__tests__/AnomalyDetectionOverview.test.tsx:101:14)
      at tryCatch (node_modules/@babel/runtime/node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/@babel/runtime/node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/@babel/runtime/node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

Two issues here:

  1. style has some incompatibilities
Type '{ alignContent?: Property.AlignContent | undefined; alignItems?: Property.AlignItems | undefined; alignSelf?: Property.AlignSelf | undefined; ... 780 more ...; vectorEffect?: Property.VectorEffect | undefined; }' is not assignable to type 'Properties<string | number, string & {}>'.
  Types of property 'MozForceBrokenImageIcon' are incompatible.
    Type 'import("/home/anan/work/OpenSearch-Dashboards/plugins/anomaly-detection-dashboards-plugin/node_modules/csstype/index").Property.MozForceBrokenImageIcon | undefined' is not assignable to type 'import("/home/anan/work/OpenSearch-Dashboards/node_modules/@types/react/node_modules/csstype/index").Property.MozForceBrokenImageIcon | undefined'.
      Type 'number & {}' is not assignable to type 'MozForceBrokenImageIcon | undefined'.
        Type 'number & {}' is not assignable to type '1'.ts(2322)
[index.d.ts(1829, 9): ]()The expected type comes from property 'style' which is declared here on type 'IntrinsicAttributes & PropsWithChildren<EuiCardProps>'

Solution would be simplify style and remove un-match parts. For example here, change to:

style={{ padding: '20px'}}
  1. Property 'betaBadgeLabel' does not exist on EuiPanel
Type '{ children: (Element | null)[]; style: { padding: string; }; className: string | undefined; betaBadgeLabel: string | undefined; }' is not assignable to type 'IntrinsicAttributes & PropsWithChildren<(DisambiguateSet<_EuiPanelButtonlike, _EuiPanelDivlike> & _EuiPanelDivlike) | (DisambiguateSet<...> & _EuiPanelButtonlike)>'.
  Property 'betaBadgeLabel' does not exist on type 'IntrinsicAttributes & PropsWithChildren<(DisambiguateSet<_EuiPanelButtonlike, _EuiPanelDivlike> & _EuiPanelDivlike) | (DisambiguateSet<...> & _EuiPanelButtonlike)>'.ts(2322)

For this one, I made an ugly fix. Not sure whether this is the best solution. I see betaBadgeLabel is a property in EuiCard and only EuiCard supports it. Meanwhile, EuiPanel is a building block component. Use it as a layout helper for containing content. It is used as a base for EuiCard. If betaBadgeLabel is required, I think we could change to EuiCard. Otherwise, could reconstruct this component. I just replace EuiPanel to EuiCard for ContentPanel component here.

const ContentPanel = (props: ContentPanelProps) => {
  const titleDataTestSubj = get(
    props,
    'titleDataTestSubj',
    'contentPanelTitle'
  );
  return (
    <EuiCard
      style={{ padding: '20px'}}
      title={""}
      className={props.contentPanelClassName}
      betaBadgeLabel={props.badgeLabel}
    >
      <EuiFlexGroup
        style={{ padding: '0px' }}
        justifyContent="spaceBetween"
        alignItems="center"
      >
        <EuiFlexItem>
          {typeof props.title === 'string' ? (
            <EuiTitle
              data-test-subj={titleDataTestSubj}
              size={props.titleSize || 's'}
              className={props.titleClassName}
            >
              <h3>{props.title}</h3>
            </EuiTitle>
          ) : (
            <EuiFlexGroup
              data-test-subj={titleDataTestSubj}
              justifyContent="flexStart"
              alignItems="center"
            >
              {Array.isArray(props.title) ? (
                props.title.map(
                  (titleComponent: React.ReactNode, idx: number) => (
                    <EuiFlexItem grow={false} key={idx}>
                      {titleComponent}
                    </EuiFlexItem>
                  )
                )
              ) : (
                <EuiFlexItem>{props.title}</EuiFlexItem>
              )}
            </EuiFlexGroup>
          )}
          <EuiFlexGroup>
            {Array.isArray(props.subTitle) ? (
              props.subTitle.map(
                (subTitleComponent: React.ReactNode, idx: number) => (
                  <EuiFlexItem
                    grow={false}
                    key={idx}
                    className="content-panel-subTitle"
                    style={{ lineHeight: 'normal', maxWidth: '75%' }}
                  >
                    {subTitleComponent}
                  </EuiFlexItem>
                )
              )
            ) : (
              <EuiFlexItem
                className="content-panel-subTitle"
                style={{ lineHeight: 'normal', maxWidth: '75%' }}
              >
                {props.subTitle}
              </EuiFlexItem>
            )}
          </EuiFlexGroup>
        </EuiFlexItem>

        <EuiFlexItem grow={false}>
          <EuiFlexGroup
            justifyContent="spaceBetween"
            alignItems="center"
            gutterSize="m"
          >
            {Array.isArray(props.actions) ? (
              props.actions.map((action: React.ReactNode, idx: number) => (
                <EuiFlexItem key={idx}>{action}</EuiFlexItem>
              ))
            ) : (
              <EuiFlexItem>{props.actions}</EuiFlexItem>
            )}
          </EuiFlexGroup>
        </EuiFlexItem>
      </EuiFlexGroup>
      {!isEmpty(props.actions) ? <EuiSpacer size="s" /> : null}
      {props.title != '' && props.hideBody !== true ? (
        <div>
          <EuiHorizontalRule
            margin="s"
            className={props.horizontalRuleClassName}
          />
          <div style={{ padding: '10px 0px' }}>
            {props.children}
          </div>
        </div>
      ) : null}
    </EuiCard>
  );
};

After all these changes, all unit tests passed:
Screen Shot 2022-03-31 at 2 53 06 PM

UI seems working
Screen Shot 2022-03-31 at 2 53 29 PM

Screen Shot 2022-03-31 at 2 53 41 PM

Again, not a UI expert, so I think my fix is a bit tmp and ugly. Just want to research on this and see they could be fixed or not. There might be other better solutions.

@tmarkley
Copy link
Contributor

We are pushing through the following PRs to address which may require changes for plugins:

#1379
#1449
#1451
#1456

@CEHENKLE
Copy link
Member

@tmarkley If those are all merged, can we close this issue?

@tmarkley
Copy link
Contributor

@CEHENKLE We’re using this as a place for plugins to reach out if they have issues with 2.0 changes. Those PRs were just a few of the recent changes that we wanted to highlight. Our plan was to keep this open for visibility until we release 2.0.

@CEHENKLE
Copy link
Member

Cool beans, thanks :)

@lguillaud
Copy link

@kavilla
Done for lguillaud/osd_transform_vis#1 (2.0.0-rc1)

@kavilla kavilla closed this as completed Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration Any plans, changes, or enhancements needed for migration nodejs 🍭 issues related to nodejs client plugins Plugin related work release >upgrade
Projects
None yet
Development

No branches or pull requests

8 participants