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

[BUG] Remove "Last updated by fields" from the UI #763

Open
AWSHurneyt opened this issue Oct 5, 2023 · 8 comments
Open

[BUG] Remove "Last updated by fields" from the UI #763

AWSHurneyt opened this issue Oct 5, 2023 · 8 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@AWSHurneyt
Copy link
Collaborator

What is the bug?
The SearchMonitor API is not returning user information on a security-enabled cluster.

How can one reproduce the bug?
Steps to reproduce the behavior:

  1. Create a monitor.
  2. Go to devtools.
  3. Execute the API call below.
  4. The response will not contain user information.
GET _plugins/_alerting/monitors/_search
{"query": {"match_all": {}}}

What is the expected behavior?
The response should contain some user information as the alerting dashboards plugin is expected to display Last updated by on two pages.

What is your host/environment?

  • OS: mac OS
  • Version: 13.4.1 (22F82)
  • Plugins: Alerting and alerting dashboards v2.9

Do you have any screenshots?
These two pages in the UI should display Last updated by; but the necessary info isn't in the API response.
Screenshot 2023-10-05 at 10 51 46 AM
Screenshot 2023-10-05 at 10 52 06 AM

Do you have any additional context?
This is the API response for the monitor referenced in the screenshots above.

GET _plugins/_alerting/monitors/_search
{"query": {"match_all": {}}}
{
  "took": 11,
  "timed_out": false,
  "_shards": {
    "total": 5,
    "successful": 5,
    "skipped": 0,
    "failed": 0
  },
  "hits": {
    "total": {
      "value": 1,
      "relation": "eq"
    },
    "max_score": 2,
    "hits": [
      {
        "_index": ".opendistro-alerting-config",
        "_id": "64ngAIsBxWW_xjH902oo",
        "_version": 5,
        "_seq_no": 6,
        "_primary_term": 1,
        "_score": 2,
        "_source": {
          "type": "monitor",
          "schema_version": 8,
          "name": "hurneyt-test updated2",
          "monitor_type": "query_level_monitor",
          "enabled": true,
          "enabled_time": 1696528317787,
          "schedule": {
            "period": {
              "interval": 1,
              "unit": "MINUTES"
            }
          },
          "inputs": [
            {
              "search": {
                "indices": [
                  "opensearch_dashboards_sample_data_ecommerce"
                ],
                "query": {
                  "size": 0,
                  "query": {
                    "bool": {
                      "filter": [
                        {
                          "range": {
                            "order_date": {
                              "from": "{{period_end}}||-1h",
                              "to": "{{period_end}}",
                              "include_lower": true,
                              "include_upper": true,
                              "format": "epoch_millis",
                              "boost": 1
                            }
                          }
                        }
                      ],
                      "adjust_pure_negative": true,
                      "boost": 1
                    }
                  }
                }
              }
            }
          ],
          "triggers": [
            {
              "query_level_trigger": {
                "id": "6ongAIsBxWW_xjH90Wpy",
                "name": "trigger1",
                "severity": "1",
                "condition": {
                  "script": {
                    "source": "ctx.results[0].hits.total.value < 10000",
                    "lang": "painless"
                  }
                },
                "actions": []
              }
            }
          ],
          "last_update_time": 1696528317787,
          "data_sources": {
            "query_index": ".opensearch-alerting-queries",
            "findings_index": ".opensearch-alerting-finding-history-write",
            "findings_index_pattern": "<.opensearch-alerting-finding-history-{now/d}-1>",
            "alerts_index": ".opendistro-alerting-alerts",
            "alerts_history_index": ".opendistro-alerting-alert-history-write",
            "alerts_history_index_pattern": "<.opendistro-alerting-alert-history-{now/d}-1>",
            "query_index_mappings_by_type": {},
            "findings_enabled": false
          },
          "owner": "alerting"
        }
      }
    ]
  }
}
@AWSHurneyt AWSHurneyt added bug Something isn't working untriaged and removed untriaged labels Oct 5, 2023
@AWSHurneyt
Copy link
Collaborator Author

Looks like this is an intentional change to the SearchMonitor API.
https://github.com/opensearch-project/alerting/pull/201/files#diff-ef8587739e7e1b5013d416eb7f9168a413b20daa72dc01f8f0e436d5bf113367R149

@zengyan-amazon recommended removing the Last updated by fields from the UI. Running this change by @kamingleung from UX.

@kamingleung
Copy link

@AWSHurneyt Removing the Last updated by makes sense. LGTM.

@AWSHurneyt
Copy link
Collaborator Author

AWSHurneyt commented Oct 11, 2023

Transferring this issue to the alerting dashboards repo since this change will be on the frontend.

New Goal

[ ] Remove the Last updated by fields from the UI (e.g., from the Monitors tab, and the monitors details page)
[ ] Backport these changes to the 1.2 and up versions.

@AWSHurneyt AWSHurneyt changed the title [BUG] SearchMonitor API not returning user information [BUG] Remove "Last updated by fields" from the UI Oct 11, 2023
@AWSHurneyt AWSHurneyt transferred this issue from opensearch-project/alerting Oct 11, 2023
@AWSHurneyt AWSHurneyt added the good first issue Good for newcomers label Oct 11, 2023
@tanupa
Copy link

tanupa commented Oct 23, 2023

I can work on this issue!

@AWSHurneyt
Copy link
Collaborator Author

I can work on this issue!

Sorry @tanupa ! Just seeing your comment now. PR #767 addresses this issue, it's just a matter of backporting it now. If you like, you could take a look at that PR to see which automatic backports failed, and manually create backport PRs for them. Otherwise, you can ignore this issue; I'm just leaving it open because the change hasn't finished backporting.

@kohinoor98
Copy link

kohinoor98 commented Nov 16, 2023

Hi @AWSHurneyt

Could I take this issue?

I have created the PRs for all the failed backports in #767

Thanks,
KC

@lezzago
Copy link
Member

lezzago commented Nov 18, 2023

@kohinoor98 please refer the original PR that solved the issue. Also when making the backports, please be cherrypick the original commit to make sure these backport commits track the original commit that fixed the problem.

@kohinoor98
Copy link

@lezzago on it! Once I will go ahead and cherry-pick the original commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants