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

thezackm/unix entity definition #14

Merged
merged 18 commits into from
Mar 25, 2021
Merged

Conversation

thezackm
Copy link
Member

Relevant information

Adding entity for Unix On-Host Integration

Checklist

  • I've read the guidelines and understand the acceptance criteria.
  • The value of the attribute marked as identifier will be unique and valid.
  • This is not my first contribution, or I've reached out to the team by opening an issue before
    opening this PR.
  • I've confirmed that my entity type wasn't already defined. If it is I'm providing an
    explanation above.

@thezackm
Copy link
Member Author

@IreneMLC @jfjoly - question came up that I'm unsure how to address. Hoping you can help:
The Unix integration covers multiple OS types, and it doesn't make a lot of sense to call them all UNIX_HOST. i.e.; if I have both AIX and MacOS in my integration, I want to visually and logically keep those separate in NROne.

Is it possible right now to create these definitions and limit them based on part of the event attributes?

Something like this perhaps?

If (SELECT osName FROM `unix:System` = 'aix'), Then apply this definition...

Ultimately I think the confusion is how granular can we get on parsing/faceting unique entities out of a comment event type.

@jfjoly
Copy link
Contributor

jfjoly commented Dec 18, 2020

@thezackm the osName can be a golden tags that will be shown in the UI. The user can create a filter to see only the entities of this type and with the tag osName='AIX'.
If we want different entity types, then we should create multiple definitions.

@IreneMLC
Copy link
Contributor

Multiple definitions could work well if we had a metric attribute that could be used like that unix:System.

definitions/infra-unix_host/definition.yml Outdated Show resolved Hide resolved
@thezackm
Copy link
Member Author

@IreneMLC

Multiple definitions could work well if we had a metric attribute that could be used like that unix:System.

do we have an example of what this would look like? I can get a PR submitted to the Unix integration to add an osVersion field (or something like it); but I'd like to see a working example first so I can make sure I'm following correctly. Thanks!

@IreneMLC
Copy link
Contributor

IreneMLC commented Dec 21, 2020

@thezackm

do we have an example of what this would look like? I can get a PR submitted to the Unix integration to add an osVersion field (or something like it); but I'd like to see a working example first so I can make sure I'm following correctly. Thanks!

What we use as a 'condition' is just another attribute in the telemetry (something like "attributeName":"attributeValue"), but I'm not familiar with the telemetry from that integration so I do not know what the actual value would be, but for example in this metric we could use the value of this attribute to know that it's related to EKS "k8s.cluster.name": "eks-nsacxp2-b6vv-prgw"

@srvaroa srvaroa requested a review from apoloa as a code owner March 10, 2021 11:32
@jfjoly
Copy link
Contributor

jfjoly commented Mar 19, 2021

@thezackm can we find a more precise condition?
As @cianbuckley said, this will match any telemetry that has hostname on it.

@thezackm
Copy link
Member Author

thezackm commented Mar 19, 2021

@thezackm can we find a more precise condition?
As @cianbuckley said, this will match any telemetry that has hostname on it.

@jfjoly - will it be possible to match on eventType() in the event stream? This will isolate the events to only Unix integration events, and then hostname would be unique in that context.

image

domain: EXT
type: UNIX_HOST
synthesis:
  name: hostname
  identifier: hostname

  conditions:
  - attribute: eventType
    prefix: unixMonitor

compositeMetrics:
  goldenMetrics:
    - ./golden_metrics.yml
  summaryMetrics:
    - ./summary_metrics.yml

Copy link
Contributor

@srvaroa srvaroa left a comment

Choose a reason for hiding this comment

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

Hi @thezackm I've sent a couple of suggestions that will make this mergeable. Adding to those, we just need to change the directory to ext-unix_host to match the new domain.

Thanks!

thezackm and others added 2 commits March 25, 2021 08:40
Co-authored-by: Galo Navarro <gnavarro@newrelic.com>
Co-authored-by: Galo Navarro <gnavarro@newrelic.com>
query:
select: max(percentUsed)
from: '`unixMonitor:Disk`'
facet: hostname, mountedOn
Copy link
Member Author

Choose a reason for hiding this comment

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

@srvaroa - is multi-facet supported on golden metrics yet?

Copy link
Contributor

Choose a reason for hiding this comment

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

@naxhh you're more up to date than me on this

Copy link
Contributor

Choose a reason for hiding this comment

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

@thezackm @naxhh tells me that it's still not supported, but no problem merging this as it is (it will not work straight away, but it's good to give us a first case to verify as soon as we complete that work).

Copy link
Contributor

Choose a reason for hiding this comment

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

@thezackm We support this in the definition but is still not being used in Golden metrics (WIP)

Check: https://github.com/newrelic-experimental/entity-synthesis-definitions/blob/main/docs/golden_metrics.md#defining-golden-metrics

So when this is implemented this will convert into: FACET entity.name, hostname, mountedOn
Since you are using hostname as the entity.name you probably want this to look like:

facet: mountedOn the facet field is now "extra facets" in the same way we use "where" as extra where aside of entity.guid IN ....

hope that makes sense!

Copy link
Member Author

Choose a reason for hiding this comment

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

perfect; I'm good with the changes and ready to go; thanks for all the support!

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, that's right @naxhh ! I forgot about that change since this branch was first made; let me take one last look through the golden and summary to validate things

Copy link
Member Author

Choose a reason for hiding this comment

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

ok; ready to merge

@sschwartzman has mentioned we will want to look at a dashboard later, but that can be added outside of this workflow

@srvaroa
Copy link
Contributor

srvaroa commented Mar 25, 2021

Merging, as the CLA bot is broken but it's already signed.

Thanks a lot for the contribution and patience!

@srvaroa srvaroa merged commit bfd9d47 into main Mar 25, 2021
@srvaroa srvaroa deleted the thezackm/unix-entity-definition branch March 25, 2021 14:42
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.

None yet

6 participants