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

Add metrics framework for frontend and backend #1306

Merged
merged 18 commits into from
Dec 2, 2022

Conversation

joshuali925
Copy link
Member

@joshuali925 joshuali925 commented Nov 30, 2022

Description

  • This PR adds the metrics counters and API to both OpenSearch and Dashboards plugins

  • Examples to instrument metrics (this PR only adds the framework and basic examples):

    • Increment OpenSearch plugin REQUEST_TOTAL counter:
      Metrics.REQUEST_TOTAL.counter.increment()
    • Increment Dashboards plugin Event Analytics create request counter:
      addRequestToMetric('event_analytics', 'create', 'count');
    • Increment Dashboards plugin Event Analytics create request error counter:
      addRequestToMetric('event_analytics', 'create', error);
    • Increment Dashboards plugin UI button event counter for { "trace_analytics": { "refresh_button": count } } in metrics response:
          <EuiButton
      +    data-click-metric-element="trace_analytics.refresh_button"
            iconType="refresh"
            onClick={props.refresh}
          >
  • To get metrics

    • OpenSearch plugin: GET http://localhost:9200/_plugins/_observability/_local/stats

      Response
      GET http://localhost:9200/_plugins/_observability/_local/stats
      
      {
        "security_permission_error": 0,
        "exception": {
          "io": 0,
          "opensearch_security": 0,
          "invalid_index_name": 0,
          "index_not_found": 0,
          "internal_server_error": 0,
          "illegal_state": 0,
          "version_conflict_engine": 0,
          "illegal_argument": 0,
          "opensearch_status": 0
        },
        "success_count": 0,
        "permission_user_error": 0,
        "request_total": 0,
        "failed_request_count_system_error": 0,
        "request_count": 0,
        "failed_request_count_user_error": 0
      }
    • Dashboards plugin: GET http://localhost:5601/api/observability/stats

      Response
      GET http://localhost:5601/api/observability/stats
      {
        "application_analytics": {
          "create": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "get": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "update": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "delete": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          }
        },
        "operational_panels": {
          "create": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "get": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "update": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "delete": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          }
        },
        "event_analytics": {
          "create": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "get": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "update": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "delete": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          }
        },
        "notebooks": {
          "create": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "get": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "update": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "delete": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          }
        },
        "trace_analytics": {
          "create": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "get": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "update": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "delete": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          }
        },
        "metrics_analytics": {
          "create": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "get": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "update": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          },
          "delete": {
            "count": 0,
            "system_error": 0,
            "user_error": 0,
            "total": 0
          }
        },
        "request_total": 0,
        "request_count": 0,
        "success_count": 0,
        "failed_request_count_system_error": 0,
        "failed_request_count_user_error": 0
      }

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2022

Codecov Report

Merging #1306 (c86df76) into 2.x (0ee5e1d) will increase coverage by 0.11%.
The diff coverage is 65.90%.

@@             Coverage Diff              @@
##                2.x    #1306      +/-   ##
============================================
+ Coverage     41.71%   41.82%   +0.11%     
- Complexity      292      315      +23     
============================================
  Files           298      302       +4     
  Lines         17753    17840      +87     
  Branches       4280     4285       +5     
============================================
+ Hits           7405     7462      +57     
- Misses        10176    10205      +29     
- Partials        172      173       +1     
Flag Coverage Δ
dashboards-observability 37.25% <ø> (ø)
opensearch-observability 70.10% <65.90%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...s/trace_analytics/components/common/search_bar.tsx 82.35% <ø> (ø)
...earch/observability/action/ObservabilityActions.kt 76.38% <0.00%> (-2.75%) ⬇️
...pensearch/observability/action/PluginBaseAction.kt 26.53% <11.11%> (-3.47%) ⬇️
...ility/resthandler/ObservabilityStatsRestHandler.kt 40.00% <40.00%> (ø)
...lity/resthandler/RestResponseToXContentListener.kt 63.63% <55.55%> (-36.37%) ⬇️
...in/org/opensearch/observability/metrics/Metrics.kt 82.14% <82.14%> (ø)
...g/opensearch/observability/metrics/BasicCounter.kt 83.33% <83.33%> (ø)
...opensearch/observability/metrics/RollingCounter.kt 94.73% <94.73%> (ø)
...rg/opensearch/observability/ObservabilityPlugin.kt 100.00% <100.00%> (ø)
...ervability/resthandler/ObservabilityRestHandler.kt 85.39% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Joshua Li <joshuali925@gmail.com>
@joshuali925 joshuali925 marked this pull request as ready for review November 30, 2022 19:41
@joshuali925 joshuali925 requested a review from a team as a code owner November 30, 2022 19:41
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
mengweieric
mengweieric previously approved these changes Dec 1, 2022
@@ -4,20 +4,21 @@
*/

import { I18nProvider } from '@osd/i18n/react';
import { QueryManager } from 'common/query_manager';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was 'common/query_manager' instead of './common/query_manager' working for you? I've seen build errors not sure if it's only on my side or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

this is from tsserver's _typescript.organizeImports command reordering lines, it shouldn't make any difference functionally

dashboards-observability/server/common/metrics/types.ts Outdated Show resolved Hide resolved
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Comment on lines +25 to +26
?.closest('[data-click-metric-element]')
?.getAttribute('data-click-metric-element')
Copy link
Collaborator

@derek-ho derek-ho Dec 2, 2022

Choose a reason for hiding this comment

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

Where is this coming from - is the idea that we label buttons with what they are doing? I see it's being added to some test snapshots, but only for trace analytics refresh? Will it be added for different other operations in the future? Nevermind, seems like you are adding this in the request itself, but why do you need to do this then? Why can't you just have it handled when we send the request?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we going to be sending these stats anywhere else other than the local index? Any place where we can get the aggregated stats?

Copy link
Member Author

Choose a reason for hiding this comment

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

@derek-ho See description. This PR only adds the framework and some basic examples.

Why can't you just have it handled when we send the request?

Instrumenting metrics in each element's onClick is tedious comparing to adding an attribute.

Are we going to be sending these stats anywhere else other than the local index? Any place where we can get the aggregated stats?

Please see description for the "To get metrics" part.

@joshuali925 joshuali925 merged commit 17b8771 into opensearch-project:2.x Dec 2, 2022
derek-ho pushed a commit that referenced this pull request Dec 22, 2022
Signed-off-by: Joshua Li <joshuali925@gmail.com>
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.

5 participants