percipia / eslgo

A GoLang FreeSWITCH ESL Library
Mozilla Public License 2.0
97 stars 42 forks source link

Change how we are formatting headers for sendevent and sendmsg #9

Closed winsock closed 3 years ago

winsock commented 3 years ago

Context

The switch away from http.Header.Write() is needed for some header strings that need to be sent to FreeSWITCH. For example the extra-headers header used in sendevent NOTIFY in certain code paths.

This change makes the following header possible: extra-headers: Messages-Waiting: yes\r\nMessage-Account: example@1.1.1.1\r\nVoice-Message: 1/0\r\n

Tradeoffs

Overview

winsock commented 3 years ago

I would love to hear what any outside users of this library think about this change. I will be leaving this open for a bit, but v1.4.1-rc1 has been tagged for anyone that needs to send line breaks for sendevent/sendmsg now.

egorsmkv commented 3 years ago

I do not have comments on this.

I hope it won't broke my programs on future updates :)

winsock commented 3 years ago

@egorsmkv My goal is to never introduce API breaking changes within major versions. Minor versions can contain new features if they don't break API compatibility. Patch versions should only contain fixes or changes that produce the same output(what this change is). At this point I do not see the need for a v2 branch anytime on the horizon.

The major impact here with this MR is potentially speed, but with my testing it seems to be within a microsecond(aka in testing error) of the standard library http.Header.Write(). It otherwise should produce the exact same output outside of allowing line feeds. In the SendEvent test for example the FormatHeaderString completes around 17µs by my measurement.

winsock commented 3 years ago

Actual Testing Results

It seems to be within margin of error on my machine. Custom Formatter:

=== RUN TestSendEvent_BuildMessage
15.098µs
--- PASS: TestSendEvent_BuildMessage (0.00s)

http.Header.Write():

=== RUN TestSendEvent_BuildMessage
14.576µs
--- PASS: TestSendEvent_BuildMessage (0.00s)

Full Results

egorsmkv commented 3 years ago

Great! Thanks for the improving of the library!