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.

Sign up for GitHub

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

Jump to bottom

Support otel metrics mapping #1397

Closed
wants to merge 66 commits into from
Closed

Support otel metrics mapping #1397

wants to merge 66 commits into from

Conversation

YANG-DB
Copy link
Member

@YANG-DB YANG-DB commented Feb 1, 2023 •
edited
Loading

Description

Simple Schema for Observability - SSO - will support the 3 major Observability types:

  • Traces
  • Logs
  • Metrics

SSO will comply with the OTEL schema and continue supporting proprietary index structure

Content

In addition to the simple schema for Observability metrics signal definition, this PR creates the following during Plugin initiation:

  • Observability Metrics index template

Issues Resolved

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.

YANG-DB added 16 commits January 27, 2023 16:14
@YANG-DB
Add Traces schema support for SSO which is OTEL compliant
326faeb 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
add basic trace samples
5d93e67 
Signed-off-by: YANGDB <[email protected]>

Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Trace-mapping' into…
3009d69 
… Support-OTEL-Trace-mapping

# Conflicts:
#	schema/traces/README.md
#	schema/traces/traces.mapping
#	schema/traces/traces.schema
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Trace-mapping' into…
7636ac5 
… Support-OTEL-Trace-mapping

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

# Conflicts:
#	schema/traces/README.md
#	schema/traces/traces.mapping
#	schema/traces/traces.schema
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Trace-mapping' into…
95a081a 
… Support-OTEL-Trace-mapping

# Conflicts:
#	schema/traces/traces.schema
@YANG-DB
add support for data-flow structure as part of the general span attri…
b0e20e0 
…butes

Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Trace-mapping' into…
6770d98 
… Support-OTEL-Trace-mapping
@YANG-DB
add support for Metrics types in Simple Schema for Observability (OTE…
cfcd2cb 
…L compliant)

Signed-off-by: YANGDB <[email protected]>
@YANG-DB
add support for Metrics types in Simple Schema for Observability (OTE…
38a8d88 
…L compliant)

Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
8872130 
…to Support-OTEL-Metrics-mapping

# Conflicts:
#	schema/metrics/README.md
#	schema/metrics/metrics.schema
@YANG-DB
add samples for the schema validation
7783f4a 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
2c3c623 
…to Support-OTEL-Metrics-mapping
@YANG-DB
fix histogram.json sample
e193ef5 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
8806dd2 
…to Support-OTEL-Metrics-mapping
@YANG-DB
add exemplar & instrumentationScope
9fe5828 
rename metrics type to kind

Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
a8163e2 
…to Support-OTEL-Metrics-mapping
kkondaka
kkondaka reviewed Feb 3, 2023
View reviewed changes
schema/metrics/histogram_metrics.mapping Outdated Show resolved Hide resolved
schema/metrics/histogram_metrics.mapping Outdated Show resolved Hide resolved
schema/metrics/histogram_metrics.mapping Outdated Show resolved Hide resolved
schema/metrics/histogram_metrics.mapping Outdated Show resolved Hide resolved
YANG-DB added 12 commits February 3, 2023 12:30
@YANG-DB
add dropped attribute count for instrumentation scope
041ffe8 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
fe27416 
…to Support-OTEL-Metrics-mapping
@YANG-DB
add dropped attribute count for instrumentation scope
1d67f61 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
c82c14a 
…to Support-OTEL-Metrics-mapping
@YANG-DB
add schemaUrl support for the outer most level
833e19f 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
2cbe27c 
…to Support-OTEL-Metrics-mapping
@YANG-DB
add template section for mapping document
436ab33 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
23c0cf5 
…to Support-OTEL-Metrics-mapping
@YANG-DB
change time to @timestamp
24aead1 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
cd9775b 
…to Support-OTEL-Metrics-mapping
@YANG-DB
rename resources to resource
9e58033 
Signed-off-by: YANGDB <[email protected]>
@YANG-DB
Merge remote-tracking branch 'origin/Support-OTEL-Metrics-mapping' in…
55b8a6e 
…to Support-OTEL-Metrics-mapping
@YANG-DB YANG-DB requested a review from kkondaka February 8, 2023 02:07
@YANG-DB YANG-DB mentioned this pull request Feb 16, 2023
Closed
@derek-ho
Copy link
Collaborator

derek-ho commented Feb 16, 2023 •
edited
Loading

@YANG-DB what if user's use data_prepper -> Opensearch without the observability plugin? Without the plugin user will not have the template and thus data stream creation would most likely fail... how would we handle this case? What i'm getting at is what if observability plugin is added later? Should we have this template live somewhere global? Is that even possible?

@YANG-DB YANG-DB requested review from ps48, kkondaka, derek-ho and penghuo and removed request for kkondaka February 17, 2023 03:08
@YANG-DB
Copy link
Member Author

YANG-DB commented Feb 17, 2023

Q: 'what if user's use data_prepper -> Opensearch without the observability plugin?'
data-prepper will need to include .../Observability/mapping/metrics-template.json inside its build so that it will be able to create the necessary index template without Observability template being installed - having in github under Observability is a good location since this repo may become the new integration repo that will contain additional integrations and schemas...

@derek-ho
Copy link
Collaborator

derek-ho commented Feb 17, 2023 •
edited
Loading

Q: 'what if user's use data_prepper -> Opensearch without the observability plugin?' data-prepper will need to include .../Observability/mapping/metrics-template.json inside its build so that it will be able to create the necessary index template without Observability template being installed - having in github under Observability is a good location since this repo may become the new integration repo that will contain additional integrations and schemas...

Ok makes sense I am not sure how that will work though... but let's create an issue either here or data prepper repo to track that edge case - where do you think it belongs?

@YANG-DB
Copy link
Member Author

YANG-DB commented Feb 17, 2023

its an issue for data-prepper...

anirudha
anirudha previously approved these changes Feb 21, 2023
View reviewed changes
@vamsimanohar
Copy link
Member

Please take care of build failures.

ps48
ps48 approved these changes Feb 22, 2023
View reviewed changes
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.

The BWC tests are failing as link to 2.6.0 artifact is not working. This may be due to build failures in jenkins ci [caused by other plugins or upstream]

vamsimanohar
vamsimanohar reviewed Mar 2, 2023
View reviewed changes
src/main/kotlin/org/opensearch/observability/index/ObservabilityIndex.kt
/**
* once lifecycle indicate start has occurred - instantiating system index creation
*/
override fun afterStart() {
Copy link
Member

Choose a reason for hiding this comment

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

When does this get triggered?
Is this after node startup or cluster startup.
If it is after node startup, is to ok to create an index if other nodes are still booting up.

Copy link
Member Author

@YANG-DB YANG-DB Mar 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is triggered from the observability plugin class that implements the cluster listener and registers the node started method - anyhow the index will not be created since the plugins are responsible of creating the index by posting data to the named index that follows the index pattern stated in the template created by this api

vamsimanohar
vamsimanohar reviewed Mar 2, 2023
View reviewed changes
src/main/kotlin/org/opensearch/observability/ObservabilityPlugin.kt
return emptyList()
}

override fun onNodeStarted() {
ObservabilityIndex.afterStart()
Copy link
Member

Choose a reason for hiding this comment

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

If we are triggering this method ourselves, why do we want to extend LifecycleListener in ObservabilityIndex class.

Copy link
Member Author

@YANG-DB YANG-DB Mar 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

correct - I'll update this...

@YANG-DB
Copy link
Member Author

YANG-DB commented Mar 9, 2023

Not relevant anymore once integration was introduced into 2.x. branch

@YANG-DB YANG-DB closed this Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vamsimanohar vamsimanohar vamsimanohar left review comments

@ps48 ps48 ps48 approved these changes

@Swiddis Swiddis Swiddis left review comments

@anirudha anirudha anirudha left review comments

@kavithacm kavithacm Awaiting requested review from kavithacm kavithacm is a code owner

@joshuali925 joshuali925 Awaiting requested review from joshuali925 joshuali925 is a code owner

@derek-ho derek-ho Awaiting requested review from derek-ho derek-ho is a code owner

@kkondaka kkondaka Awaiting requested review from kkondaka

@penghuo penghuo Awaiting requested review from penghuo penghuo is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@YANG-DB @derek-ho @vamsimanohar @anirudha @penghuo @ps48 @joshuali925 @Swiddis @kkondaka