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 agent.type field and redefine agent.name #150

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Oct 25, 2018

This is also inspired by the discussion in #142

@ruflin
Copy link
Contributor Author

ruflin commented Oct 25, 2018

@roncohen @graphaelli Would be good to get your take on this one from an APM view.

@ruflin ruflin added the review label Oct 25, 2018
@ruflin ruflin changed the title Add service.type field and redefine service.name Add agent.type field and redefine agent.name Oct 26, 2018
@ruflin ruflin force-pushed the agent-type branch 3 times, most recently from 6d704e6 to b3ad81e Compare October 30, 2018 15:11
@webmat webmat self-requested a review October 30, 2018 17:48
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

I like this emerging concept of dual fields, that allows for a custom name and a standardized name.

I wonder if name + type is the right pair of field names, however. I do think name is fine. And type is also growing on me.

Should we review the following usages of the word type elsewhere, however?

  • device.type: this is currently meant to contain a generic term like 'firewall', not a standardized (but specific) name.
  • host.type & cloud.machine.type: meant to convey a host model or virtual instance type. No concept of a host's standardized name or purpose (e.g. mysql).
  • event.type: general event classification

This is an overview of the other uses of type. I'm not saying we have to change them, I'm just surfacing a nagging worry I have, about using the word type to mean something very specific in some cases (name+type) and something else in the above cases.

@ruflin
Copy link
Contributor Author

ruflin commented Oct 31, 2018

Let's not block this PR by the above follow up discussion. I would even suggest to not touch the other type fields for now and reevaluate in a few months if we hit issues.

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Agreed, let's proceed.

@webmat webmat merged commit 38a09a2 into elastic:master Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants