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 a VisAugmenter stats API #4006

Merged

Conversation

ohltyler
Copy link
Member

@ohltyler ohltyler commented May 10, 2023

Description

Adds a server-side API /api/vis_augmenter/stats to track usage of the feature. It does this by collecting all augment-vis saved objects, and aggregating values within them. Specifically:

  • total number of saved objects
  • breakdown and count of unique origin plugins
  • breakdown and count of unique plugin resource types
  • breakdown and count of plugin resource IDs
  • breakdown and count of visualization IDs

This allows us to track what plugins and plugin resource types are being used (e.g., how many anomaly detectors are associated), as well as the individual IDs of the plugin resources and visualizations, to see if there are any unique patterns in how the feature is being used (e.g., many plugin resources to one visualization compared to roughly 1:1 mapping).

To collect these values, this PR also changes the augment-vis saved object interface to include all of these fields.

Additionally, there's some small type optimizations to the saved object loader to process the individual attributes, by introducing AugmentVisSavedObjectAttributes. Many of the functions are updated to include this interface.

Existing tests are updated and new tests are added for the functions used in the API call.

Issues Resolved

closes #3836

Check List

  • Commits are signed per the DCO using --signoff

Signed-off-by: Tyler Ohlsen <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #4006 (c51b713) into feature/feature-anywhere (03b393f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@                     Coverage Diff                      @@
##           feature/feature-anywhere    #4006      +/-   ##
============================================================
+ Coverage                     66.50%   66.52%   +0.01%     
============================================================
  Files                          3244     3245       +1     
  Lines                         62432    62459      +27     
  Branches                       9637     9640       +3     
============================================================
+ Hits                          41520    41550      +30     
+ Misses                        18604    18601       -3     
  Partials                       2308     2308              
Flag Coverage Δ
Linux 66.46% <100.00%> (+0.01%) ⬆️
Windows 66.47% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...ugmenter/public/saved_augment_vis/utils/helpers.ts 33.33% <ø> (ø)
...ter/public/saved_augment_vis/utils/test_helpers.ts 100.00% <ø> (ø)
...ter/public/saved_augment_vis/_saved_augment_vis.ts 57.14% <100.00%> (+11.68%) ⬆️
...nter/public/saved_augment_vis/saved_augment_vis.ts 100.00% <100.00%> (ø)
.../saved_augment_vis/saved_augment_vis_references.ts 93.33% <100.00%> (ø)
...ugins/vis_augmenter/server/routes/stats_helpers.ts 100.00% <100.00%> (ø)

Comment on lines +24 to +29
visName?: string;
visId?: string;
visReference?: {
id: string;
name: string;
};
Copy link
Member

Choose a reason for hiding this comment

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

Can we make a separate interface for these attributes? The convention for SavedObjectAttributes is to keep this interface identical to that of the saved object itself. If these properties are not going to be stored in the saved object, lets make a separate interface that extends this instead and use that. Also i dont see this being used anywhere, is it used in the AD plugin? if so can we just create the new interface with these properties there?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are used in the saved object actually - the visName, is populated to reference the visualization reference in the references array, and the visId and visReference are populated when pulling out the stored vis in references. This follows the same convention as used by visualization saved object extractReferences/injectReferences logic.

The visualization saved object uses the open-ended SavedObjectAttributes, and uses values there that aren't actually indexed in the saved object. I followed the same. From my understanding, the attributes can be values before/after the reference extraction/injection, based on the arguments to those functions themselves.

References:
visualization extract/inject references: link
augment-vis extract/inject references: link

Comment on lines +85 to +88
origin_plugin: originPluginMap,
plugin_resource_type: pluginResourceTypeMap,
plugin_resource_id: pluginResourceIdMap,
visualization_id: visualizationIdMap,
Copy link
Member

Choose a reason for hiding this comment

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

This is just for my understanding, but what do each of these properties mean? like whats the difference between origin_plugin, plugin_resource_type and plugin_resource_id?

Copy link
Member Author

@ohltyler ohltyler May 11, 2023

Choose a reason for hiding this comment

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

It's mentioned in the description and will be added in the docs, but will add examples here too:
origin_plugin: Anomaly Detection, Alerting
plugin_resource_type: Anomaly Detectors, Alerting Monitors
plugin_resource_id: <detector-1-id>, <monitor-1-id>

Comment on lines +56 to +57
* Given the _find response that contains all of the saved objects, iterate through them and
* increment counters for each unique value we are tracking
Copy link
Member

Choose a reason for hiding this comment

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

We should add some description of what the output exactly means. Maybe providing an example output would help clear things more.

Copy link
Member Author

Choose a reason for hiding this comment

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

So this is defined within the resp argument type - SavedObjectsFindResponse. Prefer to not add an example here which could become outdated. By looking up the defined type, it will show the fields.

Also, the saved object list is located under saved_objects field as seen on line 67

@abbyhu2000 abbyhu2000 self-assigned this May 16, 2023
@abbyhu2000 abbyhu2000 merged commit 2cc88f3 into opensearch-project:feature/feature-anywhere May 17, 2023
@ohltyler ohltyler deleted the stats-api branch May 17, 2023 22:12
@kavilla kavilla added v2.9.0 and removed v2.8.0 labels Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants