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

Log HTTP response error codes in HTTP destination #855

Merged
merged 2 commits into from
Jan 28, 2016

Conversation

MrAnno
Copy link
Collaborator

@MrAnno MrAnno commented Jan 4, 2016

Fixes: #669

@bazsi
Copy link
Collaborator

bazsi commented Jan 4, 2016

This looks good to me, I was just wondering if we could make java logs a bit more integrated to syslog-ng internal logging style.

👍

Signed-off-by: Laszlo Varady <laszlo.varady@balabit.com>
Signed-off-by: László Várady <laszlo.varady@balabit.com>
@ihrwein
Copy link
Contributor

ihrwein commented Jan 5, 2016

👍

@lbudai
Copy link
Collaborator

lbudai commented Jan 14, 2016

@bazsi: you mean that we could use
this one:
HTTP response error; error_code ='404'
instead of this:
HTTP response code error: 404
?

It's doable. @MrAnno ?

@bazsi
Copy link
Collaborator

bazsi commented Jan 14, 2016

I am not sure how java generated log messages appear in general. But yeah,
that's what I meant, and just for this log message but all of them.

This is fine, the formatting should change as a whole in a unified manner.
On Jan 14, 2016 3:13 PM, "Budai Laszlo" notifications@github.com wrote:

@bazsi https://github.com/bazsi: you mean that we could use
this one:
HTTP response error; error_code ='404'
instead of this:
HTTP response code error: 404
?

It's doable. @MrAnno https://github.com/MrAnno ?


Reply to this email directly or view it on GitHub
#855 (comment).

@MrAnno
Copy link
Collaborator Author

MrAnno commented Jan 14, 2016

Currently, the InternalMessageSender class enables to send messages with different priorities but without any tags.

JNIEXPORT void JNICALL
Java_org_syslog_1ng_InternalMessageSender_createInternalMessage(JNIEnv *env, jclass cls, jint pri, jstring message)
{
  if ((pri != org_syslog_ng_InternalMessageSender_MsgDebug) || debug_flag)
    {
      const char *c_str = (*env)->GetStringUTFChars(env, message, 0);
      msg_event_suppress_recursions_and_send(msg_event_create(pri, c_str, NULL));

      (*env)->ReleaseStringUTFChars(env, message, c_str);
    }
}

@bazsi
Copy link
Collaborator

bazsi commented Jan 14, 2016

I consider this an omission of the original implementation which should be
fixed. I wouldn't try to determine priorities when to do it, as there are
many things to improve and this is just one of them.

But java destinations should be just as integrated with syslog-ng as if
they were written in C.
On Jan 14, 2016 3:44 PM, "László Várady" notifications@github.com wrote:

Currently, the InternalMessageSender interface enables to send messages
with different priorities but without any tags.

msg_event_suppress_recursions_and_send(msg_event_create(pri, c_str, NULL));


Reply to this email directly or view it on GitHub
#855 (comment).

@MrAnno
Copy link
Collaborator Author

MrAnno commented Jan 14, 2016

Oh, I see. I will create a new issue about this.

@lbudai
Copy link
Collaborator

lbudai commented Jan 28, 2016

I'd merge this as-is, then a new PR should fix the java logmessage formatting issue.

@bazsi
Copy link
Collaborator

bazsi commented Jan 28, 2016

I agree.

Bazsi

On Thu, Jan 28, 2016 at 4:12 PM, Budai Laszlo notifications@github.com
wrote:

I'd merge this as-is, then a new PR should fix the java logmessage
formatting issue.


Reply to this email directly or view it on GitHub
#855 (comment).

@ihrwein ihrwein changed the title HTTPDestination: log HTTP response error codes Log HTTP response error codes in HTTP destination Jan 28, 2016
ihrwein added a commit that referenced this pull request Jan 28, 2016
Log HTTP response error codes in HTTP destination
@ihrwein ihrwein merged commit 1649119 into syslog-ng:master Jan 28, 2016
@ihrwein
Copy link
Contributor

ihrwein commented Jan 28, 2016

I've merged it but I forget to ask for a rebase. I hope that the master is OK.

@lbudai
Copy link
Collaborator

lbudai commented Jan 28, 2016

we will see... :-)

@bkil-syslogng
Copy link
Contributor

The integration protection that I have outlined earlier would have both caught this, and assured us whether this would be safe.

@MrAnno MrAnno added the easy label Feb 10, 2016
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.

None yet

5 participants