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

Variable naming improvement needed (when logging) #344

Open
systemcrash opened this issue Mar 11, 2020 · 4 comments
Open

Variable naming improvement needed (when logging) #344

systemcrash opened this issue Mar 11, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@systemcrash
Copy link
Contributor

When debugging, decoder.go writes e.g.:

2020/03/11 13:07:42.360377 decoder.go:149: DBG [hep] &HEP{Version:2,Protocol:17,SrcIP:10.xx.xx.xx,DstIP:10.xx.xx.xx,SrcPort:64682,DstPort:58042,Tsec:1583932062,Tmsec:356608,ProtoType:4,NodeID:101,NodePW:asdf,Payload:	i7$xܟxWܻV\sxuw|nt|xu{xwWvUux~\|u~r~|wwt|:,CID:102649YWNlMjdiNTYyMjgyMmM3Yjg2NGM1OTE3MjRlYjk3ZTc,Vlan:0,}

Some variables in the code are confusing and raise more questions than they provide answers, when used in their context - to provide information - and, IMO, should be renamed with respect to Layers for clarity:

  • Version: what version? HEP? IP?
  • Protocol: what? You mean protocol Number? How about just Transport for short or Transport_Layer_Proto.
  • ProtoType: What? If above is renamed to transport, it's more clear what this Protocol refers to, and this can be renamed to App_Layer_Proto. See table in upper right here.

How comprehensive would such a change be? (At least in local heplify-server code)

@systemcrash
Copy link
Contributor Author

Here's a few more:

heplify-server    | [102649YWNlMjdiNTYyMjgyMmM3Yjg2NGM1OTE3MjRlYjk3ZTc 2020-03-11T13:10:27.432521Z {"protocolFamily":2,"protocol":17,"srcIp":"10.48.88.2","dstIp":"10.48.254.82","srcPort":64683,"dstPort":58043,"timeSeconds":1583932227,"timeUseconds":432521,"payloadType":5,"captureId":"101","capturePass":"asdf","correlation_id":"102649YWNlMjdiNTYyMjgyMmM3Yjg2NGM1OTE3MjRlYjk3ZTc"} {"node":"101","proto":"rtcp"} {"report_count":0,"type":200,"ssrc":925155192,"sender_information":{"ntp_timestamp_sec":3792921027,"ntp_timestamp_usec":1805565597,"rtp_timestamp":1347440,"packets":1539,"octets":240000},"report_blocks":[]}]
heplify-server    |
heplify-server    | 2020/03/11 13:10:30.084516 postgres.go:294: DBG [sql] COPY hep_proto_35_default(sid,create_date,protocol_header,data_header,raw) FROM STDIN
heplify-server    |
heplify-server    | [102649YWNlMjdiNTYyMjgyMmM3Yjg2NGM1OTE3MjRlYjk3ZTc 2020-03-11T13:10:29.08056Z {"protocolFamily":2,"protocol":17,"srcIp":"10.48.88.2","dstIp":"10.48.254.82","srcPort":64682,"dstPort":58042,"timeSeconds":1583932229,"timeUseconds":80560,"payloadType":4,"captureId":"101","capturePass":"asdf","correlation_id":"102649YWNlMjdiNTYyMjgyMmM3Yjg2NGM1OTE3MjRlYjk3ZTc"} {"node":"101","proto":"4"} ~h7$x]

@systemcrash systemcrash changed the title Variable naming improvement needed Variable naming improvement needed (when logging) Mar 11, 2020
@systemcrash
Copy link
Contributor Author

To clarify: refactoring code is not a bad idea, but at this stage, I have in mind only the variables in logging output.

@negbie
Copy link
Member

negbie commented Mar 11, 2020

@systemcrash The HEP protocol is around for some time and I'm just a stranger who wanted to contribute back so I wrote the heplify tools for my company and gave them away to the sipcapture repo. The variables you mention are mostly from other places like the old kamailio hep module. I'm totally with you that the names are not the best and open for improvements/suggestions.

@negbie negbie added the enhancement New feature or request label Mar 25, 2020
@systemcrash
Copy link
Contributor Author

Well, partially, but here: #365

Will have to tackle the internal names at a later stage. Requires transitioning things that the Homer GUI may depend on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants