-
Notifications
You must be signed in to change notification settings - Fork 773
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
[Prometheus] Remove shared project and move the shared code under listener project #3503
[Prometheus] Remove shared project and move the shared code under listener project #3503
Conversation
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.Shared\PrometheusExporterOptions.cs" Link="Includes/PrometheusExporterOptions.cs" /> | ||
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.Shared\PrometheusSerializer.cs" Link="Includes/PrometheusSerializer.cs" /> | ||
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.Shared\PrometheusSerializerExt.cs" Link="Includes/PrometheusSerializerExt.cs" /> | ||
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.HttpListener\PrometheusCollectionManager.cs" Link="Includes/PrometheusCollectionManager.cs" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put these files under a folder named Shared
or Internal
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved the shared files between AspNetCore and the HttpListener projects that reside in the OpenTelemetry.Exporter.Prometheus.HttpListener
project to a folder called Shared
. And from the AspNetCore project, grouped the referenced shared files in the Include
folder.
Are the shared code actually removed from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving given it is better than the current layout. I think the public class under shared folder is something that should be fixed in the next PR.
It is now :-) |
Related to:
#3430 (comment)
#3497 (comment)
#3430 (comment)
Changes
Trying out another project setup for prometheus.
Move the files that were previously in
OpenTelemetry.Exporter.Prometheus.Shared
to be under theOpenTelemetry.Exporter.Prometheus.HttpListener
project.And removed
OpenTelemetry.Exporter.Prometheus.Shared
project and its corresponding test project.OpenTelemetry.Exporter.Prometheus.AspNetCore
now reference the shared code from the listener project.For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes