feat: LogHttpDependency() should allow status codes within 100-599 range

[mic-hnk]

LogHttpDependency() - allow status codes within the range of 100 to 599 - #516

[netlify]

✅ Deploy Preview for arcus-observability canceled.

Name	Link
🔨 Latest commit	31feeb8
🔍 Latest deploy log	https://app.netlify.com/sites/arcus-observability/deploys/63ca5b10dc72e60009687310

[request-info]

The maintainers of this repository would appreciate it if you could provide more information.

[stijnmoreels]

🎉 Thx for your contribution, @mic-hnk ! Greatly appreciated.
Some things we have to change:

• Provide new LogHttpDependency overloads with int as status code.
• Add new tests that uses this new int variant.

We can help you of course along the way!

💡 A tip for GitHub contributions: it is better to work on separate feature branches instead of on your main, otherwise conflicts and other sync problems will occur.

[fgheysels]

Thx for your contribution, @mic-hnk ! Some things we have to change:

• Revert current changes so that the current HttpStatusCode overloads still validate within the enumeration.
• Provide new LogHttpDependency overloads with int as status code.
• Add new tests that uses this new int variant.
• Updates the feature documentation to show this overload.

We can help you of course along the way!

💡 A tip for GitHub contributions: it is better to work on separate feature branches instead of on your main, otherwise conflicts and other sync problems will occur.

I don't necessarily agree with the first point @stijnmoreels , see also my comment in the issue :)

[stijnmoreels]

I don't necessarily agree with the first point @stijnmoreels , see also my comment in the issue :)

Ok, that's fine by me. Removed it. 👍
We could also see this as a fix, and do the changes in another 'feature' PR. But if @mic-hnk want to take it up, by all means! 😁

[mic-hnk]

I'm on it

[mic-hnk]

🎉 Thx for your contribution, @mic-hnk ! Greatly appreciated. Some things we have to change:

• Provide new LogHttpDependency overloads with int as status code.
• Add new tests that uses this new int variant.
• Updates the feature documentation to show this overload.

We can help you of course along the way!

💡 A tip for GitHub contributions: it is better to work on separate feature branches instead of on your main, otherwise conflicts and other sync problems will occur.

I pushed some changes.
Also, I guess this is the documentation you mean - https://observability.arcus-azure.net/features/writing-different-telemetry-types/#measuring-http-dependencies
Do you have any suggestion how I should indicate difference between previously existing methods and the int overload?

[fgheysels]

🎉 Thx for your contribution, @mic-hnk ! Greatly appreciated. Some things we have to change:

• Provide new LogHttpDependency overloads with int as status code.
• Add new tests that uses this new int variant.
• Updates the feature documentation to show this overload.

We can help you of course along the way!
💡 A tip for GitHub contributions: it is better to work on separate feature branches instead of on your main, otherwise conflicts and other sync problems will occur.

I pushed some changes. Also, I guess this is the documentation you mean - https://observability.arcus-azure.net/features/writing-different-telemetry-types/#measuring-http-dependencies Do you have any suggestion how I should indicate difference between previously existing methods and the int overload?

To be honest, I think the docs are sufficient as is. In my opinion, a developer will be able to detect this overload by using intellisense.

[stijnmoreels]

To be honest, I think the docs are sufficient as is. In my opinion, a developer will be able to detect this overload by using intellisense.

Just looked, you might be right 🤔 . We can leave it like this. 👍

[stijnmoreels]

Thank you so much for taking this up! 🥇
Some small comments but were getting there.

[stijnmoreels]

Think that this new variant you added also needs some unit tests. But otherwise this is good to go! 💯
Great job re-using the common code practices and helping us bringing higher value! Greatly appreciated. 💗

[mic-hnk]

Think that this new variant you added also needs some unit tests. But otherwise this is good to go! 💯 Great job re-using the common code practices and helping us bringing higher value! Greatly appreciated. 💗

I added some more tests :)

[fgheysels]

Thanks for this contribution.
This is a small but convenient change which will improve Arcus' usability!

[mic-hnk]

Thank you :)
This project provides a lot of value and I'm glad it's there. Keep up the good work!