Skip to content

Add scipts to monitor nginx ssl session ticket keys and session ticket r... - #7

Open
lziest wants to merge 6 commits into
openresty:masterfrom
lziest:lziest/ngx-ssl-session-ticket-keys
Open

Add scipts to monitor nginx ssl session ticket keys and session ticket r...#7
lziest wants to merge 6 commits into
openresty:masterfrom
lziest:lziest/ngx-ssl-session-ticket-keys

Conversation

@lziest

@lziest lziest commented Nov 24, 2014

Copy link
Copy Markdown

...esumptions.

@lziest

lziest commented Nov 24, 2014

Copy link
Copy Markdown
Author

change code style and file naming style. Make them consistent with existing ones.

Comment thread tapset/nginx/openssl_ex_data.sxx Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, I still don't like this module name. given that all these functions are related to OpenSSL only (and not nginx at all), you should probably just name it openssl instead of nginx.openssl_ex_data. Thoughts?

@agentzh

agentzh commented Nov 24, 2014

Copy link
Copy Markdown
Member

@lziest Also, will you add some corresponding docs to the file README.markdown?

@lziest
lziest force-pushed the lziest/ngx-ssl-session-ticket-keys branch from 941888d to 3eb5acd Compare November 24, 2014 22:38
@lziest

lziest commented Nov 24, 2014

Copy link
Copy Markdown
Author

Add docs, squashed minor changes and rebased.

Comment thread tapset/nginx/openssl.sxx Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's rename it to just openssl because this module has nothing to do with nginx.

@agentzh

agentzh commented Nov 25, 2014

Copy link
Copy Markdown
Member

Please chmod your samples/*.sxx files to executable.

Comment thread README.markdown Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please also add a line to set PATH environment, just like the docs for other tools. Thank you :)

@lziest
lziest force-pushed the lziest/ngx-ssl-session-ticket-keys branch from 72bb100 to bba6a34 Compare November 25, 2014 21:56
Comment thread README.markdown

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will you add some sample outputs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, please add sample outputs here in its docs and also some brief explanation if not too obvious.

@agentzh

agentzh commented Jan 27, 2015

Copy link
Copy Markdown
Member

@lziest Please add support for the --arg time=SECONDS option, otherwise it's good to merge :) Thanks!

Comment thread samples/ngx-ssl-session-ticket-keys.sxx Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this tool should exit here as soon as it dumps the first key set. Thoughts?

@lziest

lziest commented Jan 27, 2015

Copy link
Copy Markdown
Author

I would like to squash the commits into one once it's ok to merge

Comment thread README.markdown Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Still no indentation in the output? It's hard to look ;)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ah, still miss that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've found these ratio descriptions a bit confusing. Maybe you can make it even clearer? How about these:

Ticket resumption attempts ratio in all SSL sessions: %d
Ticket resumption success ratio in all attempts: %d
Resumed ticket sessions in all SSL sessions: %d

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.

2 participants