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

Rename input_type to type in config and input_type to prospector.type in event #4294

Merged
merged 2 commits into from
May 15, 2017

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented May 12, 2017

Document type was removed recently because _type does not exist anymore in Elasticsearch. As input_type is actually the type of the prospector it makes more sense to have it as type config options instead of type. Renaming it in the config means it should also be renamed in the event. In the event the field was renamed to prospector.type.

This change is on the config side backward compatible as input_type was only deprecated. On the event side the old field does not exist anymore.

  • Cleanup expected test json files. Indentation was standardised and input_type replaced by prospector.type.
  • Update changelog
  • Add system tests for deprecated message

@ruflin ruflin added Filebeat Filebeat in progress Pull request is currently in progress. review labels May 12, 2017
@ruflin ruflin force-pushed the input_type-to-type branch from ea3087f to c8f4ebe Compare May 12, 2017 09:07
@ruflin
Copy link
Member Author

ruflin commented May 12, 2017

jenkins, retest it

@@ -40,14 +40,14 @@ var (
)

const (
LogInputType = "log"
StdinInputType = "stdin"
LogType = "log"
Copy link
Collaborator

Choose a reason for hiding this comment

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

[golint] reported by reviewdog 🐶
exported const LogType should have comment (or a comment on this block) or be unexported

… in event

Document type was removed recently because _type does not exist anymore in Elasticsearch. As input_type is actually the type of the prospector it makes more sense to have it as type config options instead of type. Renaming it in the config means it should also be renamed in the event. In the event the field was renamed to `prospector.type`.

This change is on the config side backward compatible as input_type was only deprecated. On the event side the old field does not exist anymore.

* Cleanup expected test json files. Indentation was standardised and `input_type` replaced by `prospector.type`.
* Update changelog
* Add system tests for deprecated message
@ruflin ruflin force-pushed the input_type-to-type branch from 5081178 to ac35a91 Compare May 15, 2017 07:39
@ruflin ruflin changed the title Rename input_type to type in config and event Rename input_type to type in config and input_type to prospector.type in event May 15, 2017
@ruflin ruflin removed the in progress Pull request is currently in progress. label May 15, 2017
@@ -84,8 +85,13 @@ type config struct {

func (c *config) Validate() error {

// DEPRECATED 6.0.0: warning is already outputted on propsector level
if c.InputType != "" {
c.Type = c.InputType
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for backwards compatibility.

"program" : "GoogleSoftwareUpdateAgent",
"message" : "2016-12-13 11:35:28.420 GoogleSoftwareUpdateAgent[21412/0x700007399000] [lvl=2] -[KSAgentApp updateProductWithProductID:usingEngine:] Checking for updates for \"All Products\" using engine <KSUpdateEngine:0x100341a00\n\t\tticketStore=<KSPersistentTicketStore:0x100204520 store=<KSKeyedPersistentStore:0x100213290\n\t\t\tpath=\"/Users/tsg/Library/Google/GoogleSoftwareUpdate/TicketStore/Keystone.ticketstore\"\n\t\t\tlockFile=<KSLockFile:0x1002160e0\n\t\t\t\tpath=\"/Users/tsg/Library/Google/GoogleSoftwareUpdate/TicketStore/Keystone.ticketstore.lock\"\n\t\t\t\tlocked=NO\n\t\t\t>\n\t\t>>\n\t\tprocessor=<KSActionProcessor:0x1003bb5f0\n\t\t\tdelegate=<KSUpdateEngine:0x100341a00>\n\t\t\tisProcessing=NO actionsCompleted=0 progress=0.00\n\t\t\terrors=0 currentActionErrors=0\n\t\t\tevents=0 currentActionEvents=0\n\t\t\tactionQueue=( )\n\t\t>\n\t\tdelegate=(null)\n\t\tserverInfoStore=(null)\n\t\terrors=0\n\t>",
"timestamp" : "Dec 13 11:35:28"
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have to update these files in order for the tests to pass? From what I remember we're only comparing the fields under the module. Not that it's bad to have them updated :)

Also, how come the diffs are so big, whitespace?

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually didn't have to update them. It just passed without changes with the old type inside. But it bugged me to have "outdated" docs in the tests so I updated them manually. Then my editor automatically did the formatting. Changes are only in whitespaces and input_type to prospector.type. I can also revert it to master if you prefer.

Copy link
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

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

LGTM.

@tsg
Copy link
Contributor

tsg commented May 15, 2017

@ruflin Appveyor might have caught something.

@ruflin
Copy link
Member Author

ruflin commented May 15, 2017

@tsg Appveyor hit the race condition I just fixed in #4314

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.

3 participants