Skip to content
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

There should be a way of creating mixed case instrumentation names when using the configure_azure_monitor() convenience method. #34465

Closed
freemansoft opened this issue Feb 27, 2024 · 6 comments · Fixed by #35932
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Monitor - Exporter Monitor OpenTelemetry Exporter needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team.

Comments

@freemansoft
Copy link

freemansoft commented Feb 27, 2024

  • Package Name: azure-monitor-opentelemetry
  • Package Version: 1.2.0
  • Operating System: Windows but it is immaterial
  • Python Version: 3.9.13

Describe the bug
configure_azure_monitor() doesn't support a way of providing or later adding Metrics Views. This means that metrics change their names when moving from OpenCensus to OpenTelemetry if the old names were mixed cases with spaces.

The Python Open telemetry SDK converts all meter (gauge, counter, etc) names to lowercase.
See: open-telemetry/opentelemetry-python#3207 Experimentation shows that it fails if there are spaces in the metrics names.

The Azure OpenCensus API supported mixed case names with spaces via the view functionality.

It looks like Azure OpenTelemetry API may support mixed case via Views. It is hard to tell because there are zero examples of mixed case or spaces. This page https://learn.microsoft.com/en-us/python/api/overview/azure/monitor-opentelemetry-exporter-readme?view=azure-python-preview#metric-custom-views shows that it is possible to add views via the Meter Provider

reader = PeriodicExportingMetricReader(exporter, export_interval_millis=5000)
provider = MeterProvider(
    metric_readers=[
        reader,
    ],
    views=[
        change_metric_name_view,
    ],
)
metrics.set_meter_provider(provider)

azure.monitor.opentelmetry._confgure invokes _setupmetrics() That method does not accept or configure views. Views cannot be added once the Provider is configured.

def _setup_metrics(configurations: Dict[str, ConfigurationValue]):
    resource: Resource = configurations[RESOURCE_ARG] # type: ignore
    metric_exporter = AzureMonitorMetricExporter(**configurations)
    reader = PeriodicExportingMetricReader(metric_exporter)
    meter_provider = MeterProvider(
        metric_readers=[reader],
        resource=resource
    )
    set_meter_provider(meter_provider)

To Reproduce
Steps to reproduce the behavior:
1.

Expected behavior
The convenience method configure_azure_monitor() should accept Views or there should be a way to inject them. Or documentation should be added to say that the convenience method cannot be used for views.

Documentation should show or say whether or not the names in the views can be mixed case and include spaces as they did in the past with OpenCensus.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Monitor - Exporter Monitor OpenTelemetry Exporter needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 27, 2024
@kashifkhan
Copy link
Member

Thank you for the feedback @freemansoft . We will investigate and get back to you asap.

@kashifkhan kashifkhan removed the needs-team-triage Workflow: This issue needs the team to triage. label Feb 27, 2024
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Feb 27, 2024
@lzchen
Copy link
Member

lzchen commented Feb 27, 2024

@freemansoft

Our team is currently discussing Views configuration through the distro api at the moment. Will comment on this issue if there are any updates.

@freemansoft
Copy link
Author

You can see an example of how someone might use the current configure_azure_monitor in combination with environment variables to tune the auto-instrumented pieces. Variables for integrations and to determine if metrics, tracing, or logs get sent. Then your API to control the Azure/Microsoft specific pieces https://joe.blog.freemansoft.com/2024/02/using-opentelemetry-to-send-python.html

@lmazuel lmazuel added the Service Attention Workflow: This issue is responsible by Azure service team. label May 8, 2024
Copy link

github-actions bot commented May 8, 2024

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @jeremydvoss @lzchen.

@lzchen
Copy link
Member

lzchen commented May 9, 2024

This is currently on our agenda to work on so this is WIP.

@freemansoft
Copy link
Author

freemansoft commented Jul 8, 2024

Thanks for adding this. I finally got around to validating this by running queries against CustomMetrics and the quick check looks good

These are three of the views I configured. Note that the instrument_name has to be specified in lower case. It doesn't seem to match if it is mixed case. The actual instrument_name is in lower case even if was originally passed as mixed case. I think this is the OT standard. It was the reason I wanted the view for this specific program. Seems like a defect that the instrument_name here may not automatically toLowerCase() in the same way as the metric name was.

Here we are using the default aggregation. These are synchronous gauges so the aggregation is probably the same as the asynch gauge.

    _st_servers_time_view = SdkView(
        instrument_name="st_servers_time",
        name="ST Servers Time",
        description="get servers",
    )
    _st_best_servers_time_view = SdkView(
        instrument_name="st_best_servers_time",
        name="ST Best Servers Time",
        description="get best servers",
    )
    _st_ping_time_view = SdkView(
        instrument_name="st_ping_time",
        name="ST Ping Time",
        description="last ping",
    )

The Azure monitor query picks up the new view name (yay!) and the old non-view all-lower-case name. My new metrics are the former. The old metrics are the latter.

customMetrics | where name == "ST Ping Time" or name == "st_ping_time"

See AppInsights.py

@github-actions github-actions bot locked and limited conversation to collaborators Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Monitor - Exporter Monitor OpenTelemetry Exporter needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants