Skip to content
This repository has been archived by the owner on May 31, 2021. It is now read-only.

Add option to configure sending hostname #28

Merged
merged 3 commits into from
Dec 17, 2013
Merged

Add option to configure sending hostname #28

merged 3 commits into from
Dec 17, 2013

Conversation

danieljamesscott
Copy link

Let me know what you think. :)

@Moocar
Copy link
Owner

Moocar commented Dec 12, 2013

Could you please add some documentation to the README as well

@Moocar
Copy link
Owner

Moocar commented Dec 12, 2013

You'll also need to add host as a configurable field, and then pass it into the new instance of GelfConverter at https://github.com/Moocar/logback-gelf/blob/master/src/main/java/me/moocar/logbackgelf/GelfLayout.java#L149

@danieljamesscott
Copy link
Author

Better? :) Let me know if it needs more work.

@danieljamesscott
Copy link
Author

Any update on this? Do I need to make more changes, or is it OK to merge now?

@@ -69,6 +70,7 @@ will be the name of the thread. Defaults to false;
changed from 0.9.5 -> 0.9.6. Allowed values = 0.9.5 and 0.9.6. Defaults to "0.9.6"
* **chunkThreshold**: The maximum number of bytes allowed by the payload before the message should be chunked into
smaller packets. Defaults to 1000
* **sendinghost** The hostname of the sending host. Defaults to getLocalHostName()
Copy link
Owner

Choose a reason for hiding this comment

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

this should be hostName

@Moocar
Copy link
Owner

Moocar commented Dec 17, 2013

Hey, I've been on holiday all week. Sorry for not getting back sooner. If you can address that last comment I made, then I'll be happy to merge

@danieljamesscott
Copy link
Author

No problem. I just wanted to make sure that it didn't get forgotten. I just fixed the readme.

Moocar added a commit that referenced this pull request Dec 17, 2013
Add option to configure sending hostname
@Moocar Moocar merged commit c56e8e0 into Moocar:master Dec 17, 2013
@Moocar
Copy link
Owner

Moocar commented Dec 17, 2013

merged. Thanks for the new feature :)

@danieljamesscott
Copy link
Author

Excellent, thanks. Sorry to move straight on to the next request, do you have a timeline for the next release? It would be great to get this in the repositories.

@Moocar
Copy link
Owner

Moocar commented Dec 17, 2013

I'll let it sit for a few days. Are you using this in production? Would be
great if you could verify it. I'll aim for a release next week

On Tue, Dec 17, 2013 at 8:19 PM, Daniel Scott notifications@git.luolix.topwrote:

Excellent, thanks. Sorry to move straight on to the next request, do you
have a timeline for the next release? It would be great to get this in the
repositories.


Reply to this email directly or view it on GitHubhttps://github.com//pull/28#issuecomment-30736266
.

@danieljamesscott
Copy link
Author

I have it running on my local system, but I don't have it on any of our prod/uat stacks yet. I need to get an equivalent patch into dropwizard-gelf (https://github.com/gini/dropwizard-gelf) before I can move on... :)

@Moocar
Copy link
Owner

Moocar commented Jan 13, 2014

This is part of release 0.10p2 which should be up on maven central in a couple of hours

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants