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 otel metrics #1314

Merged
merged 21 commits into from
Jan 16, 2024
Merged

Conversation

kavithacm
Copy link
Collaborator

Description

Adding Otel metrics as a data source and also providing visualizations for it

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.

public/components/metrics/redux/slices/metrics_slice.ts Outdated Show resolved Hide resolved
public/components/metrics/redux/slices/metrics_slice.ts Outdated Show resolved Hide resolved
@@ -34,7 +34,7 @@ export const MetricsAccordion = (props: IMetricNameProps) => {
{metricsList.length > 100 && <p>Use search bar to focus listed Metrics.</p>}
<ul className="metricsList">
{metricsList.slice(0, 100).map((metric: any) => (
<li key={metric.id} className="metricsListContainer" data-test-subj={dataTestSubj}>
<li key={metric?.id} className="metricsListContainer" data-test-subj={dataTestSubj}>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is technically a dangerous safe-operator. We need to understand why the list was iterated with null/undefineds?

dispatch(deSelectMetric(metric));
};

return (
<I18nProvider>
<section className="sidebarHeight">
<DataSourcePicker
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need to look at new Sidebar. Largely, this component, with no attributes, is what will go here. None of the change above will be necessary -- all the functionality is in metricSlice!

public/components/metrics/top_menu/top_menu.tsx Outdated Show resolved Hide resolved
@ps48
Copy link
Member

ps48 commented Jan 4, 2024

@kavithacm It would be great if you could add some more context about what is being added as a feature in this PR. Maybe a link to github issue, a design RFC or can just attach a demo video.

common/constants/shared.ts Outdated Show resolved Hide resolved
common/types/metrics.ts Outdated Show resolved Hide resolved
common/types/metrics.ts Outdated Show resolved Hide resolved
Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>
@kavithacm kavithacm force-pushed the data-prepper-metrics branch from 4b0a56c to ea63152 Compare January 11, 2024 04:48
@kavithacm kavithacm marked this pull request as ready for review January 11, 2024 04:49
Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>
@@ -13,10 +14,15 @@ export interface MetricType extends VisualizationType {
w: number;
h: number;
query: {
type: 'savedCustomMetric' | 'prometheusMetric';
type: 'savedCustomMetric' | 'prometheusMetric' | typeof OTEL_METRIC_SUBTYPE;
Copy link
Member

Choose a reason for hiding this comment

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

nit. Can we move all the three in a single type

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved it, will update now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed it

import { isEmpty } from 'lodash';
import { SearchMetaData } from '../../event_analytics/redux/slices/search_meta_data_slice';
import _, { isEmpty } from 'lodash';
import { SearchMetaData } from 'public/components/event_analytics/redux/slices/search_meta_data_slice';
Copy link
Member

@ps48 ps48 Jan 11, 2024

Choose a reason for hiding this comment

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

should this be a relative path for import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure how this changed, reverted it back

@@ -21,13 +21,21 @@ import {
} from '../../../../../test/panels_constants';
import {
displayVisualization,
fetchAggregatedBinCount,
fetchSampleOTDocument,
// fetchSampleOTDocument,
Copy link
Member

Choose a reason for hiding this comment

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

can remove the commented line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed it

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>
@@ -29,6 +29,8 @@ import {
} from '../utils';
import { convertDateTime } from '../../../common/query_utils';

jest.setTimeout(60000);
Copy link
Member

Choose a reason for hiding this comment

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

let's remove this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed it

@@ -4,6 +4,8 @@
*/

import { VisualizationType } from './custom_panels';
import { OTEL_METRIC_SUBTYPE } from '../constants/shared';
type AllMetricTypes = 'savedCustomMetric' | 'prometheusMetric' | typeof OTEL_METRIC_SUBTYPE;
Copy link
Member

@ps48 ps48 Jan 11, 2024

Choose a reason for hiding this comment

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

may be MetricTypes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed it

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>
Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>
@@ -2,6 +2,7 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
/* eslint-disable @typescript-eslint/no-unused-vars */
Copy link
Member

Choose a reason for hiding this comment

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

I think this was skipped for lint check.

Copy link
Member

@ps48 ps48 left a comment

Choose a reason for hiding this comment

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

Can you please update the snapshots or rebase from main: #1352 before merging this PR? We want to see the unit tests green at the least. The snapshots are changed due to an upstream OSD change.

@kavithacm kavithacm added backport 2.x enhancement New feature or request and removed backport 2.x labels Jan 16, 2024
@kavithacm kavithacm merged commit f2a66a9 into opensearch-project:main Jan 16, 2024
9 of 23 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/dashboards-observability/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/dashboards-observability/backport-2.x
# Create a new branch
git switch --create backport/backport-1314-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f2a66a9a1c2b9dbe50b8218864e34ca570188906
# Push it to GitHub
git push --set-upstream origin backport/backport-1314-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/dashboards-observability/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1314-to-2.x.

paulstn pushed a commit to paulstn/dashboards-observability that referenced this pull request Jan 22, 2024
* List all available otel docs

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Upgraded

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* fetching bin count

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Histogram working

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Cleaned up code

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Viz working with real time data

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Successfully saving to dashboards

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Fixed sidebar jest test

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* View otel metric in dashboards

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* update snapshot and remove commented lines

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Otel metrics working end to end

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Added support for OSD dashboards

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Remove commented code

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Fixed jest test and removed console comments

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* Added test and addressed comments

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* minor

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* fix es lint

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* comments

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* change to get request

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

* remove missed commented line

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>

---------

Signed-off-by: Kavitha Conjeevaram Mohan <[email protected]>
sumukhswamy pushed a commit to sumukhswamy/dashboards-observability that referenced this pull request Jun 10, 2024
sumukhswamy added a commit that referenced this pull request Jun 11, 2024
* resolved conflicts and added the manual cherry pick for #1314

Signed-off-by: sumukhswamy <[email protected]>

* removed duplicates

Signed-off-by: sumukhswamy <[email protected]>

* updated snapshots

Signed-off-by: sumukhswamy <[email protected]>

---------

Signed-off-by: sumukhswamy <[email protected]>
Co-authored-by: Kavitha Conjeevaram Mohan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants