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

include build time and revision in version information #396

Merged
merged 2 commits into from Dec 14, 2017

Conversation

graphaelli
Copy link
Member

@graphaelli graphaelli commented Dec 12, 2017

for #375, results in:

2017/12/12 17:17:28.047058 beat.go:203: INFO Setup Beat: apm-server; Version: 7.0.0-alpha1 [6819aa06a0ed7ecc18f9bb7291d65b1ca50917fd built 2017-12-12T17:14:46]

Hook up to HTTP in #395 can happen after this is in.

note: this does add a make warning:

_beats/libbeat/scripts/Makefile:92: warning: ignoring old commands for target `apm-server'

I'm investigating whether we can get support upstream for GOBUILD_FLAGS or similar

@graphaelli graphaelli requested review from jalvz and simitt and removed request for jalvz December 12, 2017 17:31
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

LGTM
Could you please remove the TODO and rather create an Issue in the beats repo before merging.

@graphaelli
Copy link
Member Author

opened elastic/beats#5871

@graphaelli
Copy link
Member Author

@simitt elastic/beats#5871 was merged really quickly - pulled in the beats update here and eliminated the make warning - can you take another look since this changed so much since the initial review?

Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

LGTM
Could you please ensure to preserve the commit with the beats update as a seperate commit and squash the other two commits to one?

This way it will be easier to backport, as we don't want to backport the beats-update commit.

@graphaelli graphaelli merged commit 909ff76 into elastic:master Dec 14, 2017
@graphaelli graphaelli deleted the build-version branch December 14, 2017 16:12
graphaelli added a commit to graphaelli/apm-server that referenced this pull request Dec 15, 2017
elastic#396 introduced a bug where the index name contained invalid characters
that were added to the global version.
graphaelli added a commit that referenced this pull request Dec 15, 2017
#396 introduced a bug where the index name contained invalid characters
that were added to the global version.
simitt pushed a commit to simitt/apm-server that referenced this pull request Dec 18, 2017
elastic#396 introduced a bug where the index name contained invalid characters
that were added to the global version.
@ruflin
Copy link
Member

ruflin commented Dec 28, 2017

@andrewkroh I wonder if we should add this as part of libbeat especially with elastic/beats#5946 in mind?

@andrewkroh
Copy link
Member

Yes, I think we should have this in libbeat so that it becomes part of every Beat. I'd really like to have this available info in the logs and with -version.

@ruflin
Copy link
Member

ruflin commented Dec 28, 2017

@graphaelli Interested to contribute this back to beats?

@graphaelli
Copy link
Member Author

@ruflin sure, on it.

@graphaelli
Copy link
Member Author

elastic/beats#5973 opened for that @andrewkroh / @ruflin

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

4 participants