[GitHub] jmeter issue #246: Add Influxdb backend to JMeter

[pmouawad <gi...@git.apache.org>]

Github user pmouawad commented on the issue:

    https://github.com/apache/jmeter/pull/246
  
    Hello,
    Thanks for contribution.
    I have few remarks on implementation:
    - You intentionally don't set timestamp. I am not sure it's a good idea as if any delay occurs sending to InfluxDB, measurement will be wrong, and there is another reason, see next remark
    - I think HttpAsyncClient (https://hc.apache.org/httpcomponents-asyncclient-dev/quickstart.html) might be a good use case here as we don't care about responses and we don't want to block too much, so we could use it here. But in this case we would need to add timestamp.
    - I think you should allow some configuration for technical things like timeouts. InfluxdbMetricsSender#setup should have a more flexible parameter like Map to allow passing more parameters
    
    Also would it be possible to provide:
    - A Simple Test plan using the component
    - If possible some documentation
    
    I have already made some changes to PR, I could commit it as is and mark component as Alpha, we could distribute with next release and improve it either before or later. 
    @Team what do you think ?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---