micrometer-metrics / micrometer

An application observability facade for the most popular observability tools. Think SLF4J, but for observability.
https://micrometer.io
Apache License 2.0
4.46k stars 984 forks source link

Source information is not available in signalfx #1606

Open zeldigas opened 5 years ago

zeldigas commented 5 years ago

Current reporter for SignalFx does not send information about "source", that is by default equal to hostname.

SFX team's dropwizard reporter sends this information as dedicated dimension, appended to every data point: https://github.com/signalfx/signalfx-java/blob/master/signalfx-codahale/src/main/java/com/signalfx/codahale/reporter/AggregateMetricSenderSessionWrapper.java#L195

Micrometer implementation sets source field in dataPoint, but looks like it does not work as expected, because there is no dimension with a value having this info.

Expected result:

  1. It is possible to enable behavior that all reported data point has additional dimension with value equal to source() property.
  2. It is also should be allowed to define tag name with sf_ prefix so that reported data can keep compatibility with already existing data (some legacy examples for signal fx used sf_source for this info)

I'm fine if this behavior would be optional and enabled by special config parameter, e.g. management.metrics.export.signalfx.reportSource

zeldigas commented 5 years ago

I'm ready to send PR for this issue if you are fine to add this improvement

shakuzen commented 5 years ago

Right now we're just passing the configured SignalFxConfig#source as the defaultSourceName parameter to the AggregateMetricSender provided by SignalFx's Java client. I'm wondering why that doesn't result in the expected behavior and if the change belongs in Micrometer or SignalFx's Java client.


Edit: here's the specific code referenced above: https://github.com/micrometer-metrics/micrometer/blob/9d91a6b5e2d3ba0a01d9180da02b6df8e0bb4c94/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalFxMeterRegistry.java#L104-L106

zeldigas commented 5 years ago

Maybe some SFX folks could comment here. @mpetazzoni, @beccatortell, appreciate you can comment on the source field in protobuf spec compared to dimension

zeldigas commented 5 years ago

@shakuzen I did some search in SFX docs and while source field is present in protobuf descriptor, there is no such field in http api docs as well as other docs:

  1. https://developers.signalfx.com/ingest_data_reference.html - API endpoint definition to send metric data
  2. https://docs.signalfx.com/en/latest/getting-started/concepts/data-model.html - description of data model
  3. https://docs.signalfx.com/en/latest/metrics-metadata/metrics-metadata.html#metrics-metadata - various metadata about metrics only includes dimensions
zeldigas commented 5 years ago

@shakuzen do you have any comments? Are you waiting for some SignalFX rep to comment or we can count on some "de facto" behavior?

shakuzen commented 4 years ago

Sorry for the delay in response. I was taking some time off and then busy with a conference.

I think we can proceed with making the change to add the source dimension if it is not already set. I don't think it needs to be made configurable, since this seems to be the correct and expected behavior.

I would still be curious to know what is the purpose of setting the defaultSourceName on AggregateMetricSender if that info does not get sent to the server. However, we don't need to block progress on this for that.

@zeldigas Are you willing to send a pull request for this?

zeldigas commented 4 years ago

Yes, I'll send PR this week

zeldigas commented 4 years ago

PR is opened for this repo, but if I understand correctly I also need to do a follow up PR to spring-boot-actuator once this one is merged, right?

shakuzen commented 4 years ago

if I understand correctly I also need to do a follow up PR to spring-boot-actuator once this one is merged, right?

If there are additional configuration options, then yes, there will need to be a change in Spring Boot to support those.

rajanigk commented 2 years ago

Is this issue fixed? Which version of the following library it is fixed? Also should i update springboot version for this? And what property to needs to be set to get to the filter on the source in splunk apm metrics < dependency>

io.micrometer
        <artifactId>micrometer-registry-signalfx</artifactId>

< /dependency>

rajanigk commented 2 years ago

we used the splunk-otel-javaagent and are able to push metrics and filter it based on sf_service. When we push metrics using spring-boot and the micrometer-signalfx-registry library as per the springboot documentation at https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#actuator.metrics.export.signalfx, not able to filter the metrics based on the application. So thought the source dimension is for that filtering. I tried with spring-boot 2.7.2 which pulls the 1.9.2 version of micrometer-registry-signalfx the source is not coming to metric so that i could filter the metric based on it. If there is any other way to filter the metric to differentiate between different applications, please do let me know.